Promise on bug free behaviour but it creates another bug prone

[feyyazesat]

Due to the problems highlighted in this example, when holding a reference to a Value Object, it’s recommended to replace the object as a whole rather than modifying its value:

$this->price = new Money(100, new Currency('USD'));
$this->price = $this->price->increaseAmountBy(200);

This kind of behavior is similar to how basic types such as strings work in PHP. Consider the function strtolower. It returns a new string rather than modifying the original one. No reference is used; instead, a new value is returned.

The example above recommends to setting the variable which itself holds the original object. but the method is increaseAmountBy. When I read this line and when I consider PHP's natural behaviour which is object are mutuable it gives me the sense that line mutates the object. But the implementation on high level it creates new object and returns new object. This requires strong convention and in anycase it easily create bugs. Consider new developers on the team. Open source projects etc. Because this is totally a contradiction to how php works. $this->price->increaseAmountBy this gives a mutator sense. So a solution for that one don't add any mutator method to the class but do it explicitly, or adding this functionalities as static methods or may be another object to control value object.

Example :

Money::addAmount(Money $money, int $amount) : Money
{
    return new self(
        $money->getAmount() + $amount,
        $money->getCurrency()
    );
};

I like to hear your ideas ?

[keyvanakbary]

Hey @feyyazesat, thanks for your interest :)

One of the most important principles in Object-Oriented Design is encapsulation. Operating the internal structure of an object from the outside as if was a data-structure is usually a smell, this is well summarised in the Tell Don't Ask principle.

Static methods lack state and tend to make our code more procedural than Object-Oriented. Depending on the context they can be a useful tool (see meaningful constructors).

Going back to your example, is not that problematic to return new instances of the object, you actually don't have to know if the thing is mutating your object or returning a new one. This might be a matter of semantics. A good example of this is DateTimeImmutable.

As a rule of thumb, try to avoid always to expose the internal structure of your object and try to operate it from within the object itself instead of having helper methods to operate your objects from the outside (Domain vs Transaction Scripts). As a reference you can read about the typical case of a Value Object: Money Pattern.

Cheers!

[feyyazesat]

@keyvanakbary Thanks for clarity of explanation. I totally agree the example I gave it is violating fundamental part of the oop and from the design perspective it will create such a mess with all around static calls. But still I think the following example has it's drawback

$this->price = $this->price->increaseAmountBy(200);

I feel this line is a contradiction to methodname. Before figuring increaseAmountBy's implementation. It looks like oh alright it will change the object state. But instead it's immutable so will return new object with increased amount. Let's read this line in more simplistic form. Honestly I have no practical clean solution for this maybe changing method name to a getMoneyWithIncreasedAmountBy. Too long but what do you think about this case do you think that assignment with increaseAmountBy name is clear and clean ?