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?

> > In practice we expect that makeInstanceLazy*() methods will not be
> > used on fully initialized objects, and that the flag will be set most
> > of the time, but as it is the API is safe by default.
>
> In the case an object does not have a destructor, it won't make a difference 
> either way, correct?

Yes

> >> I find it interesting that your examples list DICs as a use case for 
> >> proxies, when I would have expected that to fit ghosts better.  The common 
> >> pattern, I would think, would be:
> > The RFC didn't make it clear enough that the example was about the
> > factory case specifically.
>
> Ah, got it.  That makes more sense.
>
> Which makes me ask if the $initializer of a proxy should actually be called 
> $factory?  Since that's basically what it's doing,

Good point, $factory would be a good name for this parameter.

> 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.

> >> 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);
}
```

> If it has to be a separate object, please don't make it extend 
> ReflectionClass but still give it useful dynamic methods rather than static 
> methods.  Or perhaps even do something like
>
> $ghost = new ReflectionGhostInstance(SomeClass::class, $init);
> $proxy  = new ReflectionProxyINstance(SOmeClass::class, $init);
>
> And be done with it.  (I'm just spitballing here.  As I said, I like the 
> feature, I just want to ensure the ergonomics are as good as possible.)

Thank you for your help. We will think about a better API.

Reply via email to