Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How should I handle BusinessLogic-related data definitions (like status types) in Doctrine 2?

Let's assume I have a Booking entity and it has a state field which may be set to one of a few values - let's make it: NEW, ACCEPTED, and REJECTED

I am looking for the "right" way to implement this. So far I used an approach like this:

class Booking
{
    const STATUS_NEW = 0;
    const STATUS_ACCEPTED = 1;
    const STATUS_REJECTED = 2;

    protected $status = self::STATUS_ACTIVE;
}

And it works ok, but I am really curious of the "proper" way of doing, also I have a few problems with this approach:

  1. It looks awfully lot like a business logic hidden in the entity class - if entity is supposed to be a POJO, then why would it care what the status may be? So I could put it in a manager class like:

    class BookingManager
    {
        const STATUS_NEW = 0;
        const STATUS_ACCEPTED = 1;
        const STATUS_REJECTED = 2;
    
        public function setBookingStatus(Booking $b, $status) { }
    
    }
    

    but it still does not help with the second issue:

  2. It is hard to re-use that data in the view, let's take a twig for example - I would have to create a Twig Extension in order to convert a number into an actual name:

    Status type: {{ booking.status }}
    Status name: {{ booking.status|statusName }}{# I don't like this approach #} 
    Status name: {{ booking.getStatusName() }}  {# This seems even worse #}
    

    So I could add getStatusName method to the BookingManager but I believe it does not belong there. I could add the same method to the Booking class and it would work just fine, but then again - business logic AND now also presentation logic is hidden in the entity.

  3. If some of my code depends on MyVendor\MyBundle\Entity\Booking::STATUS_NEW then could it become a problem when unit-testing the code? With static method calls the problem is obvious - I cannot mock the dependency. Is there any use case where depending on static constants may be a problem?

All I could think of would be moving all this logic to the service layer and create a service like BookingStatusManager (let's ignore the fact that it could be mistakenly took for an EntityManager subclass) - it could look like this:

class BookingStatusManager
{
    const STATUS_NEW = 0;
    const STATUS_ACCEPTED = 1;
    const STATUS_REJECTED = 2;

    public function getStatusName($code) { ... }

}

but now I need an extra class for each enum-like property for each entity and it's a pain, also it still does not seem right - I need to reference static values from this class whenever I want to deal with Booking's status.

Note that this is a general question, not one specific to presented Booking example - it could be for example BorderLength entity with lengthUnit field with possible values of MILES or KILOMETERS; also, status transitions are not restricted to those caused by users, it must be possible to perform these from the code.

What in your experience is "the right way" to solve this?

like image 389
Adam Zielinski Avatar asked Nov 21 '13 22:11

Adam Zielinski


2 Answers

I'd propose you get rid of of the constants and just create a Many-To-One association on your Booking entity with a new BookingStatus entity. Why? Well for a number of reasons:

  • Does not require editing of code if you wish to add a new booking status. Not only is this easier for developers but also allows the possibility of statuses to be dynamically created.
  • You can easily store additional information about the status in the new BookingStatus entity, e.g name. This information can also be updated without altering code.
  • Allows external tools to understand different statuses. For example you might want to use a external reporting tool directly on the database. It won't know what some integers mean but it will be able to understand a Many-To-One association.
like image 124
Tomdarkness Avatar answered Oct 05 '22 23:10

Tomdarkness


Answers to your numbered questions

1: Why wouldn't you have business logic in your business model/objects?

Coupling data and behaviour is, after all, one of the very purposes of object orientation? I believe you may have misunderstood the "POJO" concept (and I'm not talking about the J standing for Java ;)), whose purpose was to not let frameworks invade your model, thus limiting the model to a framework-specific context and making unit testing or re-use in any other context difficult.

What you're talking about sounds more like DTOs, which generally is not what your model should consist of.

2: Yes, Twig is not terribly good at manipulating numbers-that-symbolizes-meaning. You'll probably get all kinds of suggestions based on things like optimization of storage (number of bytes) or database traffic/query time, but for most projects I prefer prioritizing the human experience - i.e. don't optimize for computers unless you need it.

Thus, my personal preference (in most circumstances) is instantly dev-readable "enum" fields, but in a key-like notation instead of regular words. For example, "status.accepted" as opposed to 1 or "Accepted". Key-like notations lend themselves well to i18n, using twig's |trans filter, {% trans %} tag or something similar.

3: Static "enum" references within your model is rarely a problem while unit testing the model itself.

At some point your model needs to define its semantics anyway, through the use of the building blocks you have available. While being able to abstract away implementations (particularly of services) is useful, being able to abstract away meaning is rarely (never?) fruitful. Which reminds me of this story. Don't go there. :-D

If you're still concerned about it, put the constants in an interface that the model class implements; then your tests can reference only the interface.

 

Suggested solution

Model, alternative 1:

class Booking {
    const STATUS_NEW      = 'status.new';
    const STATUS_ACCEPTED = 'status.accepted';
    const STATUS_REJECTED = 'status.rejected';

    protected $status = self::STATUS_NEW;
}

Model, alternative 2:

interface BookingInterface {
    const STATUS_NEW      = 'status.new';
    const STATUS_ACCEPTED = 'status.accepted';
    const STATUS_REJECTED = 'status.rejected';

    // ...and possibly methods that you want to expose in the interface.
}
class Booking implements BookingInterface {
    protected $status = self::STATUS_NEW;
}

Twig:

Status name: {{ ("booking."~booking.status)|trans({}, 'mybundle') }}

(Of course, the booking. prefix is optional and depends on the way you want to structure your i18n keys and files.)

Resources/translations/mybundle.en.yml:

booking.status.new:      New
booking.status.accepted: Accepted
booking.status.rejected: Rejected

 

Constants-as-entities

On Tomdarkness' suggestion of turning these constants into their own model class, I want to stress that this should be a business/domain decision, and not a question of technical preference.

If you clearly foresee the use cases for dynamically adding statuses (supplied by the system's users), then by all means a new model/entity class is the right choice. But if the statuses are used for internal state in the application, which is coupled to the actual code you're writing (and thus won't change until the code changes too), then you're better off using constants.

Constants-as-entities makes working with them much harder ("hm, how do I get the primary key of 'accepted', again?"), isn't as easily internationalizable ("hm, do I store the possible locales as hard-coded properties on the BookingStatus entity or do I make another BookingStatusI18nStrings(id, locale, value) entity?"), plus the refactoring issue you brought up yourself. In short: don't overengineer - and good luck. ;-)

like image 44
Martin Lie Avatar answered Oct 05 '22 23:10

Martin Lie