More code comments

Posted on by Matthias Noback

Recently I read a comment on Twitter by Nikola Poša:

He was providing us with a useful suggestion, one that I myself have been following ever since reading "Clean Code" by Robert Martin. The paraphrased suggestion in that book, as well as in the tweet, is to consider a comment to be a naming issue in disguise, and to solve that issue, instead of keeping the comment. By the way, the book has some very nice examples of how comments should and should not be used.

The code comment as a naming issue

I think in most cases, indeed, we don't need comments. Common suggestions are, like Nikola suggests as well:

  • Introduce a variable with a meaningful name, which can hold the result of a complicated expression.
  • Introduce a method with a meaningful name, which can hide a piece of complicated logic.

The "hiding" that a method does, provides you with two interesting options:

  1. The method name can summarize a number of statements.
  2. The method name can represent a concept that is of a higher level than the lower-level things that are going on inside the method.

Option 1 is useful, but if you can go for option 2, it'll be the most valuable option.

Consider Nikola's example:

if ($date->hour < 9 || $date->hour > 17) { //Off-hours?
}

Option 1 would entail something like:

if ($this->hourIsBetween9And17($date->hour)) { //Off-hours?
}

Option 2 would be much more interesting and useful:

if ($this->isAnOffHour($date->hour)) {
}

This introduces abstraction, where the client doesn't care about the concrete details of determining whether or not a certain hour is an off-hour, but only wants to know if the given hour is indeed an off-hour.

#NoComments?

I realized I'm always applying the following rule: if I feel like writing a comment, I look for a way in which I can change the code so that I don't have to write a comment anymore. Also, when I encounter a comment in an existing code base, I apply the same rule, and look for ways to remove the comment. Because, as you may know, code can't lie, but comments can, so let's get rid of them before they become a risk.

However, contrary to my own advise, I also realized that I've actually been writing more and more comments over the past few months. So what's going on?

A little soul-searching revealed that I've been adding code comments that can be categorized as follows:

  1. Explanatory comments: ~70%
  2. TODO comments: ~20%
  3. WTF comments: ~10%

Explanatory comments

I've been adding explanatory comments to parts in the code that need an explanation, or a canonical description of the situation. Most often these comments can be found near a couple of statements in the domain model. The comment then explains why a certain business rule should be applied, and how the code beneath the comment actually accomplishes this. Of course, I try to match the words in the comment with the words in the code itself, so they are tied together.

Before adding an explanatory comment, I often try to rephrase the explanation as a test first. This is usually the better option, since it will make sure that the code and its documentation will never diverge. The test is the documentation that describes a piece of the code, and if the code is no longer truthful to that description, the test will fail. An even better option is to apply behavior-driven development, where tests can actually be written as stories with examples, and they will serve as automated tests at the same time.

Still, some things just need a whole paragraph of explaining, and in that case, I happily put the explanation in the code, and take my time (and a long-form commenting style like /* ... */) to explain what's going on.

"Here's a question: why don't you create a separate document, instead of a code comment?"

Good question; in my experience most developers don't care about documentation. They don't read it, don't write it, and don't update it. This means that documentation becomes untrustworthy very, very quickly. And still, nobody cares. All everybody cares about is diving into the code and making that change that needs to be done. So, knowing ourselves, I think it's much better to add a comment, than to write separate documentation.

"Okay, another question: don't you think a commit message would be more suitable for explanatory comments?

I agree, a commit message is great for a bit of explanation from your side. However, commit messages aren't as readily available as code comments are. They aren't searchable. They are not "in your face" - you don't see them automatically when you open the code. In fact, you'd have to dive into the history of that code, to figure out all the reasoning behind a piece of code. And besides, everybody would have to make small commits and write proper commit messages, which definitely doesn't happen in the real world.

By the way, I often stumble upon comments like this:

autoSelect: false, // important !!!! 

I think these are meant to be explanatory, but what they're missing is the "why" part of the explanation. Whenever you add an explanatory comment, make sure you don't forget it!

TODO comments

Besides adding explanatory comments to code, TODO comments are my favorite when doing a major refactoring, Mikado style. I sprinkle lots of these little comments across the area of the code base that I'm working on. You have to remember the one rule that should be applied when using TODO comments: when merging/releasing your work, those TODOs should be gone. This means that, as part of a code review, you should verify that no TODOs were added inside the new branch. Just like comments in general become outdated very quickly, TODO comments expire very soon as well. What happens most of the time is:

  • The TODO comment never was an intention-of-future-work, it actually was an apology in disguise.

    // TODO we should inject this service as a constructor dependency instead
    
  • The work to be done is too much and will never be done.

    // TODO rewrite using Twig
    
  • The TODO comment marks some technical debt that was introduced. The work will also never be done.

    // TODO make it possible to use this with a different tenant
    

From years of experience with TODO comments, my advice is to turn them into DONE comments right away, and move the comment to your commit message.

WTF comments

These comments are very much like TODO comments, because you're basically saying: "this ain't right", but you're also saying: "We're not going to fix this". Either because it works (even if that's a miracle), or because it would take too much time to make it right. You should still add a comment, to explain what's going on, so the next person who reads this code understands what's going on, and doesn't have to spend a lot of time trying to figure it out, just like you did now.

Conclusion

When I write a comment, I do it so that the next person who looks at this piece of code doesn't have to figure it out again. Fun fact: this is the same approach I have to many other things in life. For example, I don't want to spend time and energy over the same thought again and again, in particular if it's a troublesome thought. Same for things I have to do: if I think of it once, I write it down, so I never have to be interrupted by the thought again (until I check my TODO list, that is!).

While thinking about code comments, I became aware of an implicit rule about how I write code: I always aim to de-personify it. I try to write code that doesn't show hints of the author as a person (with emotions, habits, years of experience, etc.). When adding the types of comments discussed earlier, the code starts to have more signs of human authorship. And I actually think that's a good thing: we're working on this code together, but we're also experiencing it. Adding comments increases ownership of the code, and shows empathy towards other programmers, like our current and future team members. And to ourselves, when we dive back into the code after a holiday break. I'd say: happy commenting!

PHP clean code design Comments

Negative architecture, and assumptions about code

Posted on by Matthias Noback

In "Negative Architecture", Michael Feathers speaks about certain aspects of software architecture leading to some kind of negative architecture. Feathers mentions the IO Monad from Haskell (functional programming) as an example, but there are parallel examples in object-oriented programming. For example, by using layers and the dependency inversion principle you can "guarantee" that a certain class, in a certain layer (e.g. domain, application) won't do any IO - no talking to a database, no HTTP requests to some remote service, etc.

[...] guarantees form a negative architecture - a set of things that you know can’t happen in various pieces of your system. The potential list is infinite but that doesn’t stop this perspective from being useful. If you’re using a particular persistence technology in one part of your system, it is valuable to be able to say that it is only used in that place and no place else in the system. The same can be true of other technologies. Knowing what something is not able to do reduces the number of pitfalls. It puts architecture on solid ground.

I find that this is a great advantage of making any kind of agreement between team members: about design principles being applied, where to put certain classes, etc. Even making a clear distinction between different kinds of tests can be very useful. It's good to know that if a test is in the "unit test" directory, it won't do any IO. These tests are just some in-memory verifications of specified object behavior. So we should be able to run them in a very short amount of time, with no internet connection, no dependent services up and running, etc.

A negative architecture is what's "between the lines". You can look at all the code and describe a positive architecture for it, but given certain architecture or design rules, there's also the background; things we can be sure will never happen. Like a User entity sending an email. Or an HTML template doing a database migration. Or an HTTP client wiping out the entire disk drive. And so on and so on; I'm sure you can think of some other funny stuff that could happen (and that would require instant, nightmarish fixing).

It all sounds really absurd, but tuned down a little, these examples are actually quite realistic if you work with a legacy project that's in a bad shape. After all, these scenarios are all possible. Developers could've implemented them if they wanted to. We can never be certain that something is definitely not happening in our system. In particular if we don't have tests for that system. We can only have a strong feeling that something won't be happening.

The legacy project I'm working on these days isn't quite as bad as the ones we're fantasizing about now. But still, every now and then I stumble upon an interesting class, method, statement, which proves that my picture of the project's "negative architecture" isn't accurate enough. This always comes with the realization that "apparently, this can happen". Let's take a look at a few examples.

"An object does nothing meaningful in its constructor"

I've learned over time to "do nothing" in my constructor; just to accept constructor arguments and assign them to properties. Like this:

final class SomeService
{
    private $foo;
    private $bar;

    public function __construct($foo, $bar)
    {
        $this->foo = $foo;
        $this->bar = $bar;
    }
}

Recently, I replaced some static calls to a global translator function by calls to an injected translator service, which I added as a new constructor argument. At the last line of the constructor I assigned the translator service to a new attribute (after all, that's what I've learned to do with legacy code - add new things at the end):

final class SomeService
{
    // ...

    private $translator;

    public function __construct(..., Translator $translator)
    {
        // ...

        $this->translator = $translator;
    }
}

However, it turned out that the method that needed the (now injected) translator, was already called in the constructor:

public function __construct(..., Translator $translator)
{
    // ...
    $this->aMethodThatNeedsTheTranslator(...);

    $this->translator = $translator;
}

So before I assigned the translator to its dedicated attribute, another method attempted to use it already. This resulted in a nasty, fatal runtime error. It made me realize that my assumption was that in this project, a constructor would never do something. So even though this was a very rare case, it is something to be aware of: apparently I don't have a fully accurate picture of the "negative" architecture of the system.

By the way, the general rule that nothing gets done inside a constructor, has a related rule, which follows from it: assigning values to attributes doesn't have to happen in a specific order. I want this to be the case, since I want to have the option of swapping the order when it enhances readability.

"An object doesn't also fetch dependencies statically if it also gets any number of dependencies injected"

Related to the story about my misunderstanding of constructor arguments in some parts of the project, is the implicit rule I thought would apply to every class in the project: an object will get all its dependencies injected. I would at least like it if every object was designed like that. But I've learned to be sceptical when I encounter a service class with no (or weird) constructor arguments:

class LegacyService
{
    public function __construct()
    {
        // ...
    }
}

Since this is a service, I expect it to do something with the help of some other objects. Apparently, it doesn't get those objects injected, so I assume it fetches them from a global static place (see also my previous article "Road to dependency injection").

When looking at a class and seeing that some of its dependencies get injected, I will conclude that all of its dependencies get injected. I don't expect any of its methods to make a call to the service locator. So this is another example of negative design: we're assuming that something can never happen. I don't need to explain to you how my assumption turned out to be wrong on several occasions...

"If a class is not marked final, it has at least one subclass"

Something that has changed for me personally over the last couple of years is my use of the final keyword in front of a class declaration. There are excellent reasons for doing so, which can be summarized as: the option to extend a class should be one of its explicit use cases. It should be consciously designed to be possible to extend a class; it should not be possible by-default.

This would be an excellent example of a negative architecture: "you'll find that we never extend classes, except when they are non-final". Nowadays, when I see a "naked" class declaration, I'm assuming it will have at least one subclass. If it doesn't, also in a legacy project, I add final to it, as a first step to protect its design and prevent abuse.

Anyway, to make the distinction more clear between a class with subclasses and a class without any subclasses by design, you should make potential parent classes abstract.

"If a property is not marked "private", it's used by at least one subclass"

In the same spirit, I noticed that when I see a protected attribute, I assume that it's needed and actually used by one of the subclasses. Of course, in the real world, it turns out that often there aren't even any subclasses, but these properties are just protected by-default. If you can apply this rule to your project, it will be extremely useful. One helpful aspect is that your IDE or static analysis tool will be able to mark certain properties as unused (e.g. only used in the constructor) or as dynamically declared (e.g. not declared, but defined ad hoc).

class LegacyClass
{
    /*
     * Is this property protected on purpose or by-default?
     * The class isn't final, so... I guess it's on purpose.
     */ 
    protected $foo;
}

"An abstract class won't use methods that aren't part of its published interface"

There's a lot that I/we don't expect a class to do. One of these things is: calling a method that hasn't been defined first. The fun thing is, PHP being a dynamic language, it's totally possible to make a call in a parent class to a method that only gets defined in a subclass (or in a trait):

abstract class Foo
{
    public function foo()
    {
        $this->bar();
    }

    // bar() is strictly speaking not known to Foo
}

final class Bar extends Foo
{
    // bar() is only defined in subclasses of Foo

    public function bar()
    {
        // ...
    }
}

$foo = new Foo();
// not a problem at all!
$foo->foo();

Since PHP is a dynamic language, and everything only gets resolved at runtime, it's not a problem that bar() is an unknown method to Foo; as long as it's available at runtime. However, using this dynamic language trick isn't considered any form of reasonable practice at all, so when reading code, most of us will not even consider this a possibility.

However - you can already see it coming - I introduced a bug while refactoring a piece of code, doing exactly this, while apparently not all subclasses had this particular method. I thought this rule definitely belonged to the negative architecture of the application: no class would use a method that's not part of the published interface.

"If an object has one public method, it won't have any public static methods"

In my world, there are two types of classes:

  1. Classes which need to be instantiated and then offer one or more object or instance methods.
  2. Classes which don't need to be or can't be instantiated, but still offer one or more static methods.

I consider mixing these two characteristics bad design. However, you can do it. It's just that, nobody does it. Except when they do. Again, I introduced a regression in our code base, because I assumed all the classes to obey my classification. Again, it would be an awesome rule to be able to trust blindly.

Note that an important exception to this rule would be a named constructor, which is by definition a public static method. I find that domain objects can benefit from such a static method. So probably the first type of classes should be described as: "Classes which need to be instantiated (possibly using one or more static, named constructor methods) and then offer one or more object or instance methods." (thanks for the hint, Sylvain Robez-Masson!).

Conclusion

I think "negative architecture" is an interesting way to look at applications. Also, enumerating the above examples was a surprising exercise for me. As far as I remember, these rules aren't written down in any of the big programming books. But we still live by them, and create ourselves an image of what is and is not in our applications; what can or can't happen in them. This is very helpful, because it makes it safer for us to make changes to the code. So it would be a good idea to verify and enforce these rules on a regular basis, when doing refactoring, and while doing code reviews.

Another thought I just had: for most - if not all - of the examples above one could easily write a static analyzer, looking for cases of them in a given code base (or extend existing ones like PHP_Codesniffer or PhpStan).

PHP DDD OOP design Comments

Objects should be constructed in one go

Posted on by Matthias Noback

Consider the following rule:

When you create an object, it should be complete, consistent and valid in one go.

It is derived from the more general principle that it should not be possible for an object to exist in an inconsistent state. I think this is a very important rule, one that will gradually lead everyone from the swamps of those dreaded "anemic" domain models. However, the question still remains: what does all of this mean?

Well, for example, we should not be able to construct a Geolocation object with only a latitude:

final class Geolocation
{
    private $latitude;
    private $longitude;

    public function __construct()
    {
    }

    public function setLatitude(float $latitude): void
    {
        $this->latitude = $latitude;
    }

    public function setLongitude(float $longitude): void
    {
        $this->longitude = $longitude;
    }
}

$location = new Geolocation();
// $location is in invalid state!

$location->setLatitude(-20.0);
// $location is still in invalid state!

It shouldn't be possible to leave it in this state. It shouldn't even be possible to construct it with no data in the first place, because having a specific value for latitude and longitude is one of the core aspects of a geolocation. These values belong together, and a geolocation "can't live" without them. Basically, the whole concept of a geolocation would become meaningless if this were possible.

An object usually requires some data to fulfill a meaningful role. But it also poses certain limitations to what kind of data, and which specific subset of all possible values in the universe would be allowed. This is where, as part of the object design phase, you'll start looking for domain invariants. What do we know from the relevant domain that would help us define a meaningful model for the concept of a geolocation? Well, one of these things is that latitude and longitude should be within a certain range of values, i.e. -90 to 90 inclusive and -180 to 180 inclusive, respectively. It would definitely not make sense to allow any other value to be used. It would render all modelled behavior regarding geolocations useless.

Taking all of this into consideration, you may end up with a class that forms a sound model of the geolocation concept:

final class Geolocation
{
    private $latitude;
    private $longitude;

    public function __construct(
        float $latitude,
        float $longitude
    ) {
        Assertion::between($latitude, -90, 90);
        $this->latitude = $latitude;

        Assertion::between($longitude, -180, 180);
        $this->longitude = $longitude
    }
}

$location = new Geolocation(-20.0, 100.0);

This effectively protects geolocation's domain invariants, making it impossible to construct an invalid, incomplete or useless Geolocation object. Whenever you encounter such an object in your application, you can be sure that it's safe to use. No need to use a validator of some sorts to validate it first! This is why that rule about not allowing objects to exist in an inconsistent state is wonderful. My not-to-be-nuanced advice is to apply it everywhere.

An aggregate with child entities

The rule isn't without issues though. For example, I've been struggling to apply it to an aggregate with child entities, in particular, when I was working on modelling a so-called "purchase order". It's used to send to a supplier and ask for some goods (these "goods" are specific quantities of a certain product). The domain expert talks about this as "a header with lines", or "a document with lines". I decided to call the aggregate root "Purchase Order" (a class named PurchaseOrder) and to call the child entities representing the ordered goods "Lines" (in fact, every line is an instance of Line).

An important domain invariant to consider is: "every purchase order has at least one line". After all, it just doesn't make sense for an order to have no lines. When trying to apply this design rule, my first instinct was to provide the list of lines as a constructor argument. A simplified implementation (note that I don't use proper values objects in these examples!) would look like this:

final class PurchaseOrder
{
    private $lines;

    /**
     * @param Line[] $lines
     */
    public function __construct(array $lines)
    {
        Assertion::greaterThan(count($lines), 1,
            'A purchase order should have at least one line');

        $this->lines = $lines;
    }
}

final class Line
{
    private $lineNumber;
    private $productId;
    private $quantity;

    public function __construct(
        int $lineNumber, 
        int $productId, 
        int $quantity
    ) {
        $this->lineNumber = $lineNumber;
        $this->productId = $productId;
        $this->quantity = $quantity;
    }
}

// inside the application service:
$purchaseOrder = new PurchaseOrder(
    [
        new Line(...), 
        new Line(...)
    ]
); 

As you can see, this design makes the construction of the Line child entities a responsibility of the application service which creates the PurchaseOrder aggregate. One of the issues with that is that lines need to have an identity which is relative to the aggregate. So, when constructing these Line entities, the application service should provide it with an ID too:

// inside the application service:
$lines = [];

foreach (... as $lineNumber => ...) {
    $lines[] = new Line($lineNumber);
}

$purchaseOrder = new PurchaseOrder($lines); 

It would be cool if we wouldn't have to determine the "next identity" of a child entity outside of the aggregate. And the aggregate could happily do this work for us anyway. However, that would require a loop inside the constructor of PurchaseOrder, in which we call setLineNumber() on each Line object:

public function __construct(array $lines)
{
    foreach (array_values($lines) as $index => $line) {
        // line numbers will be 1, 2, 3, ...
        $line->setLineNumber($index + 1);
    }

    $this->lines = $lines;
}

That's not a nice solution, because now a Line can exist in an invalid, because incomplete state - without a line number.

So instead, we should let the PurchaseOrder create those Line entities itself. We'd only need to provide the raw data (product ID, quantity) as constructor arguments, e.g.

public function __construct(array $linesData)
{
    foreach (array_values($linesData) as $index => [$productId, $quantity]) {
        $this->lines[] = new Line(
            $index + 1,
            $productId,
            $quantity
        );
    }
}

However, I'm not really happy with $linesData being just some anonymous data structure. We could introduce something like a proper type for that - LineWithoutLineNumber, but that would be even more silly.

Instead, we should use a "DDD trick", that is to leave the creation of the child entity to the PurchaseOrder. We can do this using something that resembles a factory method. The difference being that this method doesn't (have to) return the created object (a Line instance), and that it also makes a state change to the PurchaseOrder. For example:

final class PurchaseOrder
{
    private $lines = [];

    public function __construct()
    {
        // no lines!
    }

    public function addLine(int $productId, int $quantity): void
    {
        $this->lines[] = new Line(
            count($this->lines) + 1,
            $productId,
            $quantity
        );
    }
}

// in the application service:
$purchaseOrder = new PurchaseOrder();
$purchaseOrder->addLine(...);
$purchaseOrder->addLine(...);
$purchaseOrder->addLine(...);

This looks great. But in the process of removing the $lines parameter from the constructor, we've unfortunately also lost our ability to protect our one domain invariant we cared about. Using this design, I can't think of a reasonable way to verify that there is at least one line. I mean, if we do that right after creating the PurchaseOrder it would be too soon. And adding this assertion to addLine() wouldn't make sense at all. The only moment at which we can reasonably verify it, is just before we persist the PurchaseOrder. In fact, we could move the validation to the repository. However, having a validate() method on PurchaseOrder wouldn't look great, and delegating the responsibility to protect domain invariants to the repository doesn't at all feel like a safe option. We'd basically be back at: throw a bunch of data at this object and validate it afterwards...

I'm tempted to go back to the first solution. But then we'd be running around in circles. Remember, if you get stuck, take a step back. We set out trying to accomplish everything in one go so we could protect this one very important domain invariant. We wouldn't even allow a nice and convenient method such as addLine() to exist, just because that allows the PurchaseOrder to exist in an incomplete, hence inconsistent state.

Being in this situation reminds me of a tweet by Alberto Brandolini:

Every modeling tool will have blind spots. Whenever the discussion around a model turns sterile, choose a different tool to challenge it.

Alberto Brandolini

We're discussing things endlessly, and we get stuck because of an object oriented programming design principle we apply without thinking. We should try to look at this PurchaseOrder thing from a different perspective. Because, if we think about it from a business perspective: a purchase order without any lines is actually fine, until it gets sent to the supplier.

A paper metaphor

As a "modelling tool" I find it very useful to imagine what dealing with a "paper purchase order" would look like. It's not at all far-fetched in this case, because even the domain experts speak of "documents". So consider how we would deal with a paper purchase order document. We'd take an empty sheet, notice some dotted lines to fill in the basics (supplier name, address, etc.). And then we see some blank space where we can write lines for every product we want to order. We could leave this paper "incomplete" for a quick bathroom stop. We can take it with us and discuss something about it with a co-worker, after which we make some corrections. Or we can even throw it away and start all over. But at some point, we're going to actually send it over to the supplier. And before we do, we give it one more look to verify that everything is there.

Translating the metaphor back to the code, we might realize that there's really two different "phases" in the life-cycle of the PurchaseOrder, which come with their own invariants. When the order is in its "draft" phase, we need to supply basic information, but we're allowed to add (and maybe remove) lines at will. Once we "finalize" the purchase order, we're claiming that it's ready to be sent, and at that point we could protect some other invariants.

We would only need to add a method to PurchaseOrder that would "finalize" it. Trying to be DDD-compliant, we look for a word that our domain experts use. This word turns out to be "place" - we're gradually filling in all the details and then we place the purchase order. So, in code it could look something like this:

final class PurchaseOrder
{
    private $lines = [];
    private $isPlaced;

    public function __construct(...)
    {
        // ...
    }

    public function addLine(int $productId, int $quantity): void
    {
        if ($this->isPlaced) {
            throw new \LogicException(
                'You cannot add a line to an order that was already placed'
            );
        }

        $this->lines[] = new Line(
            count($this->lines) + 1,
            $productId,
            $quantity
        );
    }

    public function place(): void
    {
        Assertion::greaterThan(count($this->lines), 1,
            'A purchase order should have at least one line');

        $this->isPlaced = true;
    }
}

// in the application service:
$purchaseOrder = new PurchaseOrder();

$purchaseOrder->addLine(...);
$purchaseOrder->addLine(...);
$purchaseOrder->addLine(...);

$purchaseOrder->place();

Note that, besides adding a place() method, we also modified addLine() to prevent new lines from being added after the order was placed. In the paper metaphor this wouldn't be allowed either, since the document has been sent to the supplier, so it will be very confusing if lines get added in our local version of the purchase order.

Also note that the place() method brings the aggregate root in a certain state, after which not everything is possible anymore. This might remind you of the concept of a state machine. I actually find that entities are often much like state machines. Given a certain state, operations on it are limited. And state transitions are limited too. For example, before placing the order, it would be possible to cancel it without any consequences, but after placing it, the system needs to take all kinds of compensating actions (send a message to the supplier that the order has been cancelled, etc.).

Conclusion

I find that leaving the exact type and construction details of nested objects to their parent object leads to a more "supple" design. In a sense, this is "old OOP knowledge" - we hide the implementation details of how exactly the PurchaseOrder deals with the lines (e.g. does it use a plain old array, or a collection object, do we need a Line class at all, etc.). We thereby allow refactoring of the PurchaseOrder aggregate without having to update all its clients across the code base.

This is part of what's meant by the traditional DDD advice to make the aggregate root the only entry point to interaction with any aggregate part:

Choose one ENTITY to be the root of each AGGREGATE, and control all access to the objects inside the boundary through the root.

Eric Evans, "Domain-Driven Design", Part II, Chapter Six: "The Life Cycle of a Domain Object"

No client should be able to make use of or create new Line objects directly. Unless, I have to add, there is a very specific use case for that. For instance, you should not expose all those internals for the sake of unit testing (see also a previous article - "Testing actual behavior").

So even though we ended up with a better design, we had to reconsider options for protecting the "an order should have at least one line" domain invariant. We discussed reaching for other modelling tools, like working out a "paper metaphor". Of course there are other modelling tools, but this one was effective for making us realize there are actually two distinct phases in the life-cycle of the purchase order.

The general advice being: if you find yourself stuck with a modelling question, look for ways to change your perspective. Even look for (unconsciously) applied (programming) rules that keep you from reaching the best solution. Alberto adds another useful suggestion to that:

...and you’re maybe looking for "the best" solution. When all you need is "a solution".

PHP DDD OOP design Comments