Hello Valentin,

Thanks for having a look.

> Arnaud and I are pleased to share with you the RFC we've been shaping for
>> over a year to add native support for lazy objects to PHP.
>>
>> Please find all the details here:
>> https://wiki.php.net/rfc/lazy-objects
>>
>> We look forward to your thoughts and feedback.
>>
>> Cheers,
>> Nicolas and Arnaud
>>
> Hi, Nicolas and Arnaud!
>
> I like the idea, thank you for the RFC!
>
> Here are some initial thoughts and questions:
>
> 1. It doesn't seem right that calling
> `ReflectionLazyObject::makeLazyGhost` has an implicit side effect on
> `$instance` and returns reflection. It does 2 things and thus breaks the
> SRP. Having smth like `$lazyGhost = new
> ReflectionClass(MyClass)->newLazyGhost($initializer)` and/or
> `ReflectionLazyObject::makeLazy($object, $initializer): void` seems better.
>

About "new ReflectionClass(MyClass)->newLazyGhost", we could add this but
it would provide a duplicate way to achieve what can be done with
"makeLazy($object)" variants. As you might have read in the RFC, being able
to make a pre-existing instance lazy is needed to cover all use cases.

Then about "ReflectionLazyObject::makeLazy($object, $initializer): void",
is the difference only to return void and not a ReflectionLazyObject
instance? Then this might provide an underperforming API: creating a lazy
object and immediately after setting some of its properties is an use case
that can happen on the hot path (of e.g. Doctrine entities creation steps).
Returning "void" would force an extra call to
ReflectionLazyObject::fromInstance() that the proposed API prevents.


> 2. If `ReflectionLazyObject extends ReflectionObject`, then how `new
> ReflectionLazyObject($object)` will work for non-lazy objects? Will it
> throw?
>

The constructor is private so this is not allowed (I should add this to the
RFC).


> 3. Is extending `ReflectionObject` really necessary? What about creating
> `ReflectionLazyObject` as a standalone class without abusing inheritance?
> Or simply adding methods to `ReflectionObject` / `ReflectionClass`?
>

I don't think extending ReflectionObject is necessary. I don't know if
doing so is "abusing" inheritance. It might make sense either way. For the
use cases I identified, it wouldn't harm to not extend anything. Does
anyone else see a reason to go in one or the other direction? To me it just
makes sense to have ReflectionLazyObject extend ReflectionObject.

About adding methods to `ReflectionObject` / `ReflectionClass`, you mention
SRP in your message; ReflectionClass/ReflectionObject is already crowded,
and this lazy object topic is better separated to me.

4. The RFC says that Virtual state-proxies are necessary because of
> circular references. It's difficult to accept this reasoning, because using
> circular references is a bad practice and the given example is something I
> try to avoid by all means in my code.
>

Yet circular references happen all the time in any non-trivial app, so this
has to be supported.

Nicolas

Reply via email to