On Sat, 26 Nov 2022 at 23:06, Marco Pivetta <ocram...@gmail.com> wrote:
> On Sat, 26 Nov 2022, 23:56 G. P. B., <george.bany...@gmail.com> wrote: > >> As all the invariants will be maintained in the child classes >> > The invariant here is the immutability from a behavioural PoV. > LSP has 100% been broken in that example. > I've gone back to the example, and if we are going to use bad code to "argue" against certain features, then let's use this argument to its natural conclusion. But before that, we really need to get everyone clear on what LSP is. It is a subtyping relationship that states: if a property P on an object X of type T is provable to be true, then for an object Y of type S to be considered a proper subtype of T, this property P must also be true for all object Y of type S. Let's rephrase this slightly in terms more appropriate to PHP: if a characteristic P on an instance X of class T is provable to be true, then for an instance Y of class S to be considered a proper subclass of T, this characteristic P must also be true for all instances Y of class S. Having this in mind, let me show how easy it is to break LSP: class T { public function __construct(public readonly int $i) {} public function add(self $t): self { return new self($this->i + $t->i); } } $op1 = new T(5); $op2 = new T(14); $r = $op1->add($op2); var_dump($r); class S extends T { public function add(parent $t): self { return new self($this->i - $t->i); } } $op1 = new S(5); $op2 = new T(14); $r = $op1->add($op2); var_dump($r); Clearly, the characteristic that we can "add two T together" has been broken in class S. This is all to say, the type system can only provide a level of code *correctness*, and that remains even in highly sophisticated type systems. If you bear with me and the introduction of the following imaginary type dependent syntax: Matrix<n*m> which declares a Matrix type which is an n by m matrix. Thus, we can write the signature of a function that does define matrix multiplication in the following way: function multiplication(Matrix<n*m> $op1, Matrix<m*k> $op2): Matrix<n*k> Which would guarantee us that the 2 matrices provided can indeed be multiplied together as they have compatible dimensions, and it even gives us the dimensions of the output matrix. However, the one thing it cannot guarantee is that the operation we are actually doing is a matrix multiplication, I could just as well write a function that returns a new n by k matrix filled with zeroes. How can one be sure that a subclass will not alter the behaviour? By making the method final. That's mainly why personally I think final by default, and using composition over inheritance, is "better". And I think from what we've seen in language design over the last couple of decades is the OO inheritance is just a bad way to do subtyping and something like Haskell type-classes are way better. [1] But that ship has somewhat sailed for PHP. The LSP checks PHP (and for the matter any type system) can do and does, is checking the logical correctness of the implementation, not behavioural correctness that onus still remains on the developer. Now, let's have another look at your example: /* readonly */ class ImmutableCounter { public function __construct(private readonly int $count) {} public function add1(): self { return new self($this->count + 1); } public function value(): int { return $this->count; } } IMHO the fact that the count property is private is the biggest issue, just changing it to protected would solve most of the issues, as now you've given the tools to PHP to verify and ensure this constraint holds for the whole class hierarchy. But more importantly, it means I don't need to reimplement the *whole* parent class and (risk me messing up)/(keeping in sync) the behaviour of the parent class if I want to do something like: class SubstractableImmutableCounter extends ImmutableCounter { public function sub1(): self { return new self($this->count - 1); } } As it's easy to mess up even just writing a function properly that does the expected behaviour, I suppose we should remove the ability for users to write them, as they cannot be trusted with. So please, can we stop using the argument that this proposal breaks LSP if you write dumb code, as that's already the case, and it doesn't if you provide PHP with the actual information it needs to verify in child classes. Moreover, if one expects a readonly class to be immutable, then that is already not the case if one of the properties is a non-readonly object. It is also not necessarily stateless, as can be seen in the following example: https://3v4l.org/A2Lbs readonly class ImmutableBadNumberGenerator { public function __construct(private readonly int $i) {} public function generate(): int { static $j = 1; ++$j; return $this->i + $j; } } $o = new ImmutableBadNumberGenerator(10); var_dump($o->generate()); var_dump($o->generate()); var_dump($o->generate()); Now what *is* up to debate is what the expectation around a class extending a readonly class is. And that I don't really have strong opinions. However, what this discussion has told me more is that the expectation around readonly is more like the immutable keyword in D than the behaviour of the readonly keyword in other languages (as PHP and C# is the same AFAIK). Best regards, George P. Banyard [1] https://diogocastro.com/blog/2018/06/17/typeclasses-in-perspective/