Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123507
Precedence: bulk
MIME-Version: 1.0
References: <CAOWwgpmbq5VRrZQvUXDsKiNK4r6+bFA4VxnjQ_U=h8T9r0o3DA@mail.gmail.com>
 <CAPyeZ4NR7dVBuereYb0r4P3PRYF8y_pJcQLqEXazDQFKGqn9xQ@mail.gmail.com>
In-Reply-To: <CAPyeZ4NR7dVBuereYb0r4P3PRYF8y_pJcQLqEXazDQFKGqn9xQ@mail.gmail.com>
Date: Tue, 4 Jun 2024 18:56:43 +0200
Message-ID: <CAOWwgpnRNgYX7a3GeMq7E2MU0sSrdm9PWoDAE5rBOXhMg0qxVQ@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Lazy Objects
To: Valentin Udaltsov <udaltsov.valentin@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>, Arnaud Le Blanc <arnaud.lb@gmail.com>
Content-Type: multipart/alternative; boundary="00000000000062425f061a13569c"
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--00000000000062425f061a13569c
Content-Type: text/plain; charset="UTF-8"

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

--00000000000062425f061a13569c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div>Hello Valentin,</div><div><br></div>=
<div>Thanks for having a look.<br></div></div><div class=3D"gmail_quote"><b=
lockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-le=
ft:1px solid rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div class=
=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px =
0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir=
=3D"ltr"><div dir=3D"ltr"><p>Arnaud and I are pleased to share with
 you the RFC we&#39;ve been shaping for over a year to add native support=
=20
for lazy objects to PHP.</p><p>Please find all the details here:<br><a rel=
=3D"noreferrer" href=3D"https://wiki.php.net/rfc/lazy-objects" target=3D"_b=
lank">https://wiki.php.net/rfc/lazy-objects</a></p><p>We look forward to yo=
ur thoughts and feedback.</p><p>Cheers,<br>Nicolas and Arnaud</p></div></di=
v>
</blockquote></div>Hi, Nicolas and Arnaud!<br clear=3D"all"><div><br>I like=
 the idea,=C2=A0thank you for the=C2=A0RFC!<br><br></div><div>Here are some=
 initial thoughts and questions:<br><br>1. It doesn&#39;t seem right that c=
alling `ReflectionLazyObject::makeLazyGhost` has an implicit side effect on=
 `$instance` and returns reflection. It does 2 things and thus breaks the S=
RP. Having smth like `$lazyGhost =3D new ReflectionClass(MyClass)-&gt;newLa=
zyGhost($initializer)` and/or `ReflectionLazyObject::makeLazy($object, $ini=
tializer): void` seems better.<br></div></div></blockquote><div><br></div><=
div>About &quot;new ReflectionClass(MyClass)-&gt;newLazyGhost&quot;, we cou=
ld add this but it would provide a duplicate way to achieve what can be don=
e with &quot;makeLazy($object)&quot; variants. As you might have read in th=
e RFC, being able to make a pre-existing instance lazy is needed to cover a=
ll use cases.</div><div><br></div><div>Then about &quot;ReflectionLazyObjec=
t::makeLazy($object, $initializer): void&quot;, is the difference only to r=
eturn void and not a ReflectionLazyObject instance? Then this might provide=
 an underperforming API: creating a lazy object and immediately after setti=
ng some of its properties is an use case that can happen on the hot path (o=
f e.g. Doctrine entities creation steps). Returning &quot;void&quot; would =
force an extra call to ReflectionLazyObject::fromInstance() that the propos=
ed API prevents.<br></div><div>=C2=A0</div><blockquote class=3D"gmail_quote=
" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);=
padding-left:1ex"><div dir=3D"ltr"><div></div><div>2. If `ReflectionLazyObj=
ect extends ReflectionObject`, then how `new ReflectionLazyObject($object)`=
 will work for non-lazy objects? Will it throw?<br></div></div></blockquote=
><div><br></div><div>The constructor is private so this is not allowed (I s=
hould add this to the RFC).<br></div><div>=C2=A0</div><blockquote class=3D"=
gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(20=
4,204,204);padding-left:1ex"><div dir=3D"ltr"><div>3. Is extending=C2=A0`Re=
flectionObject` really necessary? What about creating `ReflectionLazyObject=
` as a standalone class without abusing inheritance? Or simply adding metho=
ds to `ReflectionObject` / `ReflectionClass`?</div></div></blockquote><div>=
<br></div><div>I don&#39;t think extending ReflectionObject is necessary. I=
 don&#39;t know if doing so is &quot;abusing&quot; inheritance. It might ma=
ke sense either way. For the use cases I identified, it wouldn&#39;t harm t=
o not extend anything. Does anyone else see a reason to go in one or the ot=
her direction? To me it just makes sense to have ReflectionLazyObject exten=
d ReflectionObject.</div><div>=C2=A0</div><div>About adding methods to `Ref=
lectionObject` / `ReflectionClass`, you mention SRP in your message; Reflec=
tionClass/ReflectionObject is already crowded, and this lazy object topic i=
s better separated to me.</div><div><br></div><blockquote class=3D"gmail_qu=
ote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,20=
4);padding-left:1ex"><div dir=3D"ltr"><div>4. The RFC says that=C2=A0Virtua=
l state-proxies are necessary because of circular references. It&#39;s=C2=
=A0difficult to accept=C2=A0this reasoning, because using circular=C2=A0ref=
erences is a bad=C2=A0practice and the given example is something=C2=A0I tr=
y to avoid by all means in my code.<br></div></div></blockquote><div><br></=
div><div>Yet circular references happen all the time in any non-trivial app=
, so this has to be supported.</div><div><br></div><div>Nicolas<br></div></=
div></div>

--00000000000062425f061a13569c--