Hi Larry,
On Fri, Jun 14, 2024 at 10:18 PM Larry Garfield <la...@garfieldtech.com> wrote:
> > The actual instance is allowed to escape the proxy and to create direct
> > references to itself.
>
> How? Is this a "return $this" type of situation? This could use more
> fleshing out and examples.
"return $this" will return the proxy object, but it is possible to
create references to the actual instance during initialization, either
directly in the initializer function, or in methods called by the
initializer. The "About Proxies" section discusses this a bit. I've
added an example.
> The terms "virtual" and "proxy" seem to be used interchangeably in different
> places, including in the API. Please just use one, and purge the other.
> It's confusing as is. :-) (I'd favor "proxy", as it seems more accurate to
> what is happening.)
Agreed
> Under Common Behavior, you have an example of calling the constructor
> directly, using the reflection API, but not of binding the callable, which
> the text says is also available. Please include an example of that so we can
> evaluate how clumsy (or not) it would be.
I've clarified that binding can be achieved with Closure::bind(). 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
> > After calling newLazyGhostInstance(), the behavior of the object is the
> > same as an object created by newLazyGhostInstance().
>
> I think the first is supposed be a make* call?
Thank you!
> > When making an existing object lazy, the makeInstanceLazy*() methods call
> > the destructor unless the SKIP_DESTRUCTOR flag is given.
>
> I don't quite get why this is. Admittedly destructors are rarely used, but
> why does it need to call the destructor?
The rationale is that unless specified otherwise, we must assume that
the constructor has been called on the object. Therefore we must call
the destructor before resetting the object's state entirely. See also
the Mutex example given by Tim. I've added the rationale and an
example.
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.
> 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:
>
> class Service {
> public function __construct(private ServiceA $a, private ServiceB $b) {}
> }
>
> $c = some_container();
>
> $init = fn() => $this->__construct($c->get(ServiceA::class),
> $c->get(ServiceB::class));
>
> $service = new ReflectionLazyObjectFactory(Service::class, $init);
>
> (Most likely in generated code that can dynamically sort out the container
> calls to inline.)
>
> Am I missing something?
No you are right, but they must fallback to the proxy strategy when
the user provides a factory.
E.g. this will use the ghost strategy because the DIC
instantiates/initializes the service itself:
my_service:
class: MyClass
arguments: [@service_a, @service_b]
lazy: true
But this will use the proxy strategy because the DIC doesn't
instantiate/initialize the service itself:
my_service:
class: MyClass
arguments: [@service_a, @service_b]
factory: [@my_service_factory, createService]
lazy: true
The RFC didn't make it clear enough that the example was about the
factory case specifically.
> 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
