On 9/1/2016 3:49 PM, Silvio Marijić wrote: > Hi Andre, > > Here is RFC https://wiki.php.net/rfc/immutability and you have link to > implementation github. Any suggestions and feedback are more then welcome. > > Best, > Silvio >
Hi Silvio,
very nice work you guys did here! :)
Abstract classes are not mentioned at all in the RFC. However, there is
a test case from which it is clear that abstract classes cannot be
immutable. Are there any reasons for this restrictions?
What about array and resource values? Not mentioned in the RFC.
The fact that cloning is not possible should also be extended in the
RFC. I mean, it's clear to me but maybe not to others. Remember that the
RFC is the main source of information for the feature (e.g. to generate
documentation).
Why the restrictions that all properties of an immutable class that take
objects must be immutable too? It's clear why an immutable property must
contain an immutable class but the inheritance from the class to the
properties is not consistent with how things work. An immutable class
might want to contain an internal cache (e.g. flyweight pattern).
immutable final class Flyweight {
private static $instances = [];
public immutable $value;
private function __construct($value) {
$this->value = $value;
}
public static function ENUM_ORD() {
if (isset(self::$instances[1]) === false) {
self::$instances[1] = new self(1);
}
return self::$instances[1];
}
}
$o1 = Flyweight::ENUM_ORD();
$o2 = Flyweight::ENUM_ORD();
var_dump($o1 === $o2); // bool(true)
Note that we could add the restriction that an immutable class that
should be used in a threading context must contain only immutable
properties in the future when the need arises. However, for now I do not
see the need to inherit the modifier from the class to its properties
and I see use cases where the developer wants more control.
The test cases cover the most stuff but not everything and could be
extended. There are other things with the PR but I will check it out and
create a PR against your branch with that so you can review it. (Might
take a while so bare with me.)
The RFC contains several grammatical issues. I could help you with that
too if you want.
There is a lot of diff noise in the ext/tokenizer/tokenizer_data.c file.
--
Richard "Fleshgrinder" Fussenegger
signature.asc
Description: OpenPGP digital signature
