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:
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:
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.
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?
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:
BookingStatus
entity, e.g name. This information can also be updated without altering code.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.
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
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. ;-)
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With