Hi Larry,
Following your feedback we propose to amend the API as follows:
```
class ReflectionClass
{
public function newLazyProxy(callable $factory, int $options): object {}
public function newLazyGhost(callable $initializer, int $options): object {}
public function resetAsLazyProxy(object $object, callable
$factory, int $options): void {}
public function resetAsLazyGhost(object $object, callable
$initializer, int $options): void {}
public function initialize(object $object): object {}
public function isInitialized(object $object): bool {}
// existing methods
}
class ReflectionProperty
{
public function setRawValueWithoutInitialization(object $object,
mixed $value): void {}
public function skipInitialization(object $object): void {}
// existing methods
}
```
Comments / rationale:
- Adding methods on ReflectionClass instead of ReflectionObject is
better from a performance point of view, as mentioned earlier
- Keeping the word "Lazy" in method names is clearer, especially for
"newLazyProxy" as a the "Proxy" pattern has many uses-cases that are
not related to laziness. However we removed the word "Instance" to
make the names shorter.
- We have renamed "make" methods to "reset", following your feedback
about the word "make". It should better convey the behavior of these
methods, and clarify that it's modifying the object in-place as well
as resetting its state
- setRawValueWithoutInitialization() has the same behavior as
setRawValue() (from the hooks RFC), except it doesn't trigger
initialization
- Renamed $initializer to $factory for proxy methods
WDYT?
Best Regards,
Arnaud
On Sun, Jun 16, 2024 at 3:46 PM Arnaud Le Blanc <arnaud...@gmail.com> 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?
>
> > > 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.
