On 05/04/2024 01:20, Saki Takamachi wrote:
I don't think it will be possible to make a good Money class as a child of
this. BcNum is a read-only class, so if the constructor of BcNum is final then
the child class won't be able to take the currency as a constructor param, and
won't be able to protect the invariant that currency must be initialized. If
the constructor of BcNum were made non-final then BcNum wouldn't be able to
protect the invariant of the numeric value always being initialized once the
constructor has returned. And it should be straightforward to make a good money
class as a composition of BcNum and a currency enum or string.
The RFC does not list the constructor as final, and I would not expect it to.
There's probably not any good use case for a subclass to add properties, unless
perhaps the subclass developers are willing do do away with some of the checks
that would normally be done on a value object by the PHP runtime to keep it
valid (maybe replacing them with static analysis checks). But there are lots of
reasonable use cases for subclasses to add methods, even if they're effectively
syntax sugar for static methods with a BcNum typed param.
I literally just provided in the quote you are replying to a good use case for
a subclass. You can do the same thing with composition yeah, and that might
even be better to a lot of people, but I don't think this RFC should take a
position AGAINST subclasses and in favor of composition.
Having just written that I've now checked the behavior of DateTime -
seehttps://3v4l.org/5DQZg. The constructor of that isn't final, so a child
class can replace it with an empty constructor. But if it does that and leaves
the value uninitialized then it blows up on a call to `format`. That's probably
not something we should emulate. DateTimeImmutable behaves the same way.
Why not? Writing something like that obviously does not work, and the error
would be immediately apparent. Is it possible to create something that will
error? Of course. That's not an error that needs to be aggressively guarded
against, because the feedback is rapid and obvious.
I don't think protecting initialization by the constructor is
necessary.https://3v4l.org/YroTN
This problem can occur with any class, not just `DateTime`. And I think this is
the responsibility of the user's code, and the language just needs to throw an
exception appropriately.
To clarify: In my opinion, I think that value objects provided by languages
are not intended to force immutability or design onto users, to provide
functionality to users who expect them to behave as immutable value objects.
Therefore, even if a child class that inherits from `Number` behaves in a way
that is not immutable, it is the responsibility of the user and there is no
need to prohibit it.
In other words, the concerns we are currently aware of are issues of
responsibility on the user's side, and my opinion is that there is no need to
give that much consideration. However, this is my personal opinion, and there
may be more reasonable opinions.
Ah ok. Maybe that's the standard way internal classes are written, and
we can consider it generally an error for a child class to override the
constructor without calling `parent::__construct()`. I know tools warn
users not to forget the parent call.
And I had been thinking of BcNum as a readonly class, but I see it's not
an readonly class, it's a mutable class with its one and only property
being readonly. I agree that it seems unnecessary to force child
classes to be immutable - I wasn't concerned about them adding
additional, mutable, properties, only about leaving the value property
uninitialized. (Btw I would have supported the 1st part of the 2022
Readonly amendments RFC, which I think would have made a readonly class
behave the same as a class where every property is readonly,
particularly since would have allowed implementing the example given on
the Wikipedia page for LSP: a circle with fixed centre and mutable
radius may inherit from an immutable point)
