Hi
On 6/4/24 14:28, Nicolas Grekas wrote:
Please find all the details here:
https://wiki.php.net/rfc/lazy-objects
We look forward to your thoughts and feedback.
I've gave the RFC three or four passes and I'm not quite sure if I
follow everything, here's a list of some questions / remarks that came
to mind, roughly ordered by the order of things appearing in the RFC.
- "been tested successfully on the Doctrine and on the Symfony projects"
Is there a PoC patch showcasing how the code would change / be
simplified for those pre-existing codebases?
- int $options = 0
Not a fan of flag parameters that take a bitset, those provide for a
terrible DX due to magic numbers. Perhaps make this a regular (named)
parameter, or an list of enum LazyObjectOptions { case
SkipInitOnSerialize; }?
- skipProperty()
Not a fan of the method name, because it doesn't really say what it
does, without consulting the docs. Perhaps `skipInitializationFor()` or
similar?
- setProperty()
Not a fan of the method name, because it is not a direct counterpart to
`getProperty()`. Unfortunately I don't have a better suggestion.
- The examples should be expanded and clarified, especially the one for
makeLazyProxy():
My understanding is that the $object that is passed to the first
parameter of makeLazyProxy() is completely replaced. Is this
understanding correct? What does that mean for spl_object_hash(),
spl_object_id()? What does this mean for WeakMap and WeakReference? What
does this mean for objects that are only referenced from within $object?
Consider this example:
class Foo {
public function __destruct() { echo __METHOD__; }
}
class Bar {
public string $s;
public ?Foo $foo;
public function __destruct() { echo __METHOD__; }
}
$bar = new Bar();
$bar->foo = new Foo();
ReflectionLazyObject::makeLazyProxy($bar, function (Bar $bar) {
$result = new Bar();
$result->foo = null;
$result->s = 'init';
return $result;
});
var_dump($bar->s);
My understanding is that this will dump `string(4) "init"`. Will the
destructor of Foo be called? Will the destructor of Bar be called?
- What happens if I make an object lazy that already has all properties
initialized? Will that be a noop? Will that throw? Will that create a
lazy object that will never automatically be initialized?
- Cloning, unless __clone() is implemented and accesses a property.
The semantics of cloning a lazy object should be explicitly spelled out
in the RFC, ideally with an example of the various edge cases (should
any exist).
- Before calling the initializer, properties that were not initialized
with ReflectionLazyObject::skipProperty(),
ReflectionLazyObject::setProperty(),
ReflectionLazyObject::setRawProperty() are initialized to their default
value.
Should skipProperty() also skip the initialization to the default value?
My understanding is that it allows skipping the initialization on
access, but when initialization actually happens it should probably be
set to a well-defined value, no?
Am I also correct in my understanding that this should read "initialized
to their default value (if any)", meaning that properties without a
default value are left uninitialized?
- If an exception is thrown while calling the initializer, the object is
reverted to its pre-initialization state and is still considered lazy.
Does this mean that the initializer will be called once again when
accessing another property? Will the "revert to its pre-initialization"
work properly when you have nested lazy objects? An example would
probably help.
- The initializer is called with the object as first parameter.
What is the behavior of accessing the object properties, while the
initializer is active? Based on the examples, I assume it will not be
recursively called, similarly to how the hooks work?
- The object is marked as non-lazy and the initializer is released.
What does it mean for the initializer to be released? Consider the
following example:
ReflectionLazyObject::makeLazyGhost($o, $init = function ($o) use
(&$init) {
$o->init = $init;
});
- The return value of the initializer has to be an instance of a parent
or a child class of the lazy-object and it must have the same properties.
Would returning a parent class not violate the LSP? Consider the
following example:
class A { public string $s; }
class B extends A { public function foo() { } }
$o = new B();
ReflectionLazyObject::makeLazyProxy($o, function (B $o) {
return new A();
});
$o->foo(); // works
$o->s = 'init';
$o->foo(); // breaks
- The destructor of lazy non-initialized objects is not called.
That sounds unsafe. Consider the following example:
class Mutex {
public string $s;
public function __construct() {
// take lock
}
public function __destruct() {
// release lock
}
}
$m = new Mutex();
ReflectionLazyObject::makeLazyGhost($m, function ($m) {
});
unset($m); // will not release the lock.
- Using the Manager::createManager() factory is not compatible with
ghost objects because their initializer requires initializing the ghost
object in place,
I don't understand that example, because it doesn't actually use lazy
objects and thus I don't understand if Manager, Dispatcher, or both are
intended to be lazily initialized. It would help to rewrite the example
to use `makeLazyGhost()` and indicate with a comment in which cases the
problem would arise.
- Backward Incompatible Changes
There a technicality: The `ReflectionLazyObject` class name will no
longer be available to userland.
Best regards
Tim Düsterhus
