On Sun, Jun 16, 2024, at 8:46 AM, Arnaud Le Blanc wrote: > On Sat, Jun 15, 2024 at 7:13 PM Larry Garfield <la...@garfieldtech.com> wrote: >> > In practice I expect there will be two kinds of ghost initializers: >> > - Those that just call one public method of the object, such as the >> > constructor >> > - Those that initialize everything with ReflectionProperty::setValue() >> > as in the Doctrine example in the "About Lazy-Loading strategies" >> > section >> I'm still missing an example with ::bind(). Actually, I tried to write a >> version of what I think the intent is and couldn't figure out how. :-) >> >> $init = function() use ($c) { >> $this->a = $c->get(ServiceA::class); >> $this->b = $c->get(ServiceB::class); >> } >> >> $service = new ReflectionLazyObjectFactory(Service::class, $init); >> >> // We need to bind $init to $service now, but we can't because $init is >> already registered as the initializer for $service, and binding creates a >> new closure object, not modifying the existing one. So, how does this even >> work? > > Oh I see. Yes you will not be able to bind $this in a simple way here, > but you could bind the scope. This modified example will work: > > ``` > $init = function($object) use ($c) { > $object->a = $c->get(ServiceA::class); > $object->b = $c->get(ServiceB::class); > } > $service = new ReflectionLazyObjectFactory(Service::class, > $init->bindTo(null, Service::class)); > ``` > > If you really want to bind $this you could achieve it in a more convoluted > way: > > ``` > $init = function($object) use ($c) { > (function () use ($c) { > $this->a = $c->get(ServiceA::class); > $this->b = $c->get(ServiceB::class); > })->bindTo($object)(); > } > $service = new ReflectionLazyObjectFactory(Service::class, $init); > ``` > > This is inconvenient, but the need or use-case is not clear to me. > Could you describe some use-cases where you would hand-write > initializers like this? Do you feel that the proposal should provide > an easier way to change $this and/or the scope?
Primarily I was just reacting to this line: > However, for more complex use-cases where the initializer wishes to access > non-public properties, it is required to bind the initializer function to the > right scope (with Closure::bind()), or to access properties with > ReflectionProperty. And asking "OK, um, how?" Because what I was coming up with didn't make any sense. :-) It's debatable if the use case of wanting to assign to private properties in the initializer without using Reflection is common enough to warrant more. I'm not sure at this point. If we wanted to, I could see an extra flag that would tell the system to bind the closure to the object before calling it. I don't know how common a need that will be, though, so I won't insist it be included. But I would like to see that line clarified with one of the above examples, because as is, I would expect to be able to bind to the object as I was trying to do and it (obviously) didn't work. >> and I'm unclear what it would do with the proxy object itself that's passed >> in. > > Passing the factory itself as argument could be used to make decisions > based on the value of some initialized field, or on the class of the > object, or on its identity. I think Nicolas had a real use-case where > he detects clones based on the identity of the object: > > ``` > $init = function ($object) use (&$originalObject) { > if ($object !== $originalObject) { > // we are initializing a clone > } > }; > $originalObject = $reflector->newProxyInstance($init); > ``` > > This was on ghosts, but I think it's also a valid use-case example for > proxies. Hm, interesting. Please include this sort of example in the RFC, so we know what the use is (and when it won't matter, which seems like it would be the more common case). >> >> ReflectionLazyObjectFactory is a terrible name. Sorry, it is. :-) >> >> Especially if it's subclassing ReflectionClass. If it were its own >> >> thing, maybe, but it's still too verbose. I know you don't want to put >> >> more on the "dumping ground" fo ReflectionClass, but honestly, that feels >> >> more ergonomic to me. That way the following are all siblings: >> >> >> >> newInstance(...$args) >> >> newInstanceWithoutConstructor(...$args) >> >> newGhostInstance($init) >> >> newProxyInstance($init) >> >> >> >> That feels a lot more sensible and ergonomic to me. isInitialized(), >> >> initialized(), etc. also feel like they make more sense as methods on >> >> ReflectionObject, not as static methods on a random new class. >> > >> > Thank you for the suggestion. We will check if this fits the >> > use-cases. Moving some methods on ReflectionObject may have negative >> > performance implications as it requires creating a dedicated instance >> > for each object. Some use-cases rely on caching the reflectors for >> > performance. >> > >> > Best Regards, >> > Arnaud >> >> I'm not clear why there's a performance difference, but I haven't looked at >> the reflection implementation in, well, ever. :-) > > What I meant is that creating an instance (not necessarily of > ReflectionObject, but of any class) is more expensive than just doing > nothing. The first two loops below would be fine, but the last one > would be slower. This can make an important difference in a hot path. > > ``` > foreach ($objects as $object) { > ReflectionLazyObject::isInitialized($object); > } > > $reflector = new ReflectionClass(SomeClass::class); > foreach ($objects as $object) { > $reflector->isInitialized($object); > } > > foreach ($objects as $object) { > $reflector = new ReflectionObject($object); > $reflector->isInitialized($object); > } > ``` Ah, now I see what you mean. Interesting. Including that reasoning in the RFC would be good. Though, I don't know how often I'd be calling isInitialized() on a larger set of objects, hot path or no. --Larry Garfield