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?

Answers


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. ;-)


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.

I use very simple approach. Example:

class Offer extends Entity implements CrudEntityInterface
{
    const STATUS_CANCELED = -1;
    const STATUS_IN_PROGRESS = 0;
    const STATUS_FINISHED = 1;
    const STATUS_CONFIRMED = 2;

    protected static $statuses_names = [
        self::STATUS_CANCELED => 'canceled',
        self::STATUS_IN_PROGRESS => 'in_progress',
        self::STATUS_FINISHED => 'finished',
        self::STATUS_CONFIRMED => 'confirmed'
    ];

    /**
     * @var integer
     *
     * @ORM\Column(name="status", type="integer")
     * @Assert\NotBlank
     */
    protected $status = self::STATUS_IN_PROGRESS;

    public static function getStatuses()
    {
        return self::$statuses_names;
    }

    /**
     * Set status
     *
     * @param integer $status
     * @return Offer
     */
    protected function setStatus($status)
    {
        if(!array_key_exists($status, self::$statuses_names)){
            throw new \InvalidArgumentException('Status doesn\'t exist');
        }

        $this->status = $status;

        return $this;
    }

    /**
     * Get status
     *
     * @return integer 
     */
    public function getStatus()
    {
        return $this->status;
    }

    public function getStatusName()
    {
        return self::$statuses_names[$this->status];
    }
}

All display names are always translated, in order to keep separation from model

{{ ('offer.statuses.'~offer.statusName)|trans }}

While it's not the most elegant, you can get a little pragmatic, and just use string keys in a fairly efficient, safe, way:

<?php

class Booking
{

    protected $statusMap = array(
        0 => 'new',
        1 => 'accepted',
        2 => 'rejected'
    );

    /**
     * A premature optimization, trading memory to reduce calls to
     * array_flip/array_search in a more naive implementation
     *
     * @var array
     */
    protected $statusMapReverse;

    public function __construct(){
        $this->statusMapReverse = array_flip($this->statusMap);
        $this->setStatus('new');
    }

    /**
     * @ORM\Column(type="integer")
     */
    protected $status;

    /**
     * @param string $status Valid values are 'new', 'accepted', and 'rejected'
     *
     * @throws InvalidBookingStatusException
     */
    public function setStatus($status){
        if (! in_array($status, $this->statusMap)){
            throw new InvalidBookingStatusException();
        }
        $this->status = $this->statusMapReverse[$status];
    }

    /**
     * @return string
     */
    public function getStatus(){
        return $this->statusMap[$this->status];
    }

    /**
     * @return integer
     */
    public function getStatusCode(){
        return $this->status;
    }

    /**
     * @return array
     */
    public function getStatusMap(){
       return $this->statusMap;
    }

}

I find I use this pattern pretty frequently when I need to model this kind of enumeration-like data. It has a few nice features:

1) Status is stored as in integer in the db.

2) Calling code doesn't ever need to care about those (integer) values, unless it wants to.

3) Calling code is protected against typos/invalid status since setStatus validates strings (can only be checked at runtime, but hey)

4) Booking::getStatusMap() makes it easy to generate select boxes, etc.

You could expand setStatus to accept either a string or integer (status-code), and still validate, if you wanted to be able to set status by code.

The disadvantages (compared to using constants), are mainly:

1) Querying by status becomes a bit of a hassle. Client code needs to getStatusMap() and find codes.

2) IDEs won't be quite as helpful in telling calling code what valid statuses are (though the phpdoc annotation on setStatus() can help, if you keep it up-to-date.

3) Invalid status throws an exception at runtime, while with constants you find out at compile-time.

--

Final note, I don't see the need for a BookingManager at all. Yes, entities are just plain objects, but that doesn't mean they can't contain logic to manage their own state.


I like handling those kind of issues in a generic way (a bit of an overkill if all you need is a status, but what project ever needed only one enum-like field?);

First, I create a "GenericOptionEntity", having the following fields:

  • ID
  • Tag
  • Label
  • Entity
  • Field

And a one to many relationship to associate the entity (or the base entity, in case of a generic "status") to the appropriate field.

The "entity" and "field" fields help identify the proper options for form types.

The GenericOptionRepository should implement a "getOptionByTag", so that you wouldn't have to use IDs to get the proper entity. Also, a generic method for getting appropriate options for the calling method \ field.

Lastly (depending on the case), a bunch of interfaces sometimes come in handy, especially for making entities "Statusable" when you need it.

I really like this approach, mostly because:

  • It makes sense :)
  • Adding and removing options can be done by an admin, via GUI
  • Easy to filter the appropriate options for the view / controller tiers
  • Allows re-use of all the helper methods required
  • Does not clutter your project with mini-classes created for 4 DB rows

I would go with Tom's approach and make the status table look like this :

  • id
  • description
  • isAccepted (bool)
  • isRejected (bool)

And the values like this :

  • id : 1
  • description : New
  • isAccepted : 0
  • isRejected : 0

  • id : 2

  • description : Accepted
  • isAccepted : 1
  • isRejected : 0

  • id : 3

  • description : Rejected
  • isAccepted : 0
  • isRejected : 1

You then query with isAccepted or isRejected without having to worry about the id.


I recently worked out something similar in a MenuItem entity and three constant 'types'.

The approach I like includes adding some additional 'is' methods. I placed them right in the entity, but that approach is up to you. I'm not sure there is a single 'right way'.

<?php

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

    protected $status;

    public function setBookingStatus($status)
    {
        $this->status = $status;
    }

    public function getStatus()
    {
        return $this->status;
    }

    public function isStatusNew()
    {
        return $this->status === self::STATUS_NEW;
    }

    public function isStatusAccepted()
    {
        return $this->status === self::STATUS_ACCEPTED;
    }

    public function isStatusRejected()
    {
        return $this->status === self::STATUS_REJECTED;
    }
}

That allows you to use some easier logic in your Twig templates (YMMV):

{% if booking.statusAccepted %}

{% elseif booking.statusRejected %}

{% else %}

In this way you can keep a similar Twig syntax to your constants.


Need Your Help

jQuery $.position() on Safari & Chrome

jquery svg raphael

I have an issue using $.position to retrieve the relative x/y offset on an element.