Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114641
To: internals@lists.php.net
References: <CAOwTYAtc_xD6n=efmamkAy4ro4KqPKxq18-zXsNriY0280FY2g@mail.gmail.com>
Message-ID: <319c4288-5934-b218-d6eb-0b787a2cdef1@gmx.net>
Date: Thu, 27 May 2021 15:55:50 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.0
MIME-Version: 1.0
In-Reply-To: <CAOwTYAtc_xD6n=efmamkAy4ro4KqPKxq18-zXsNriY0280FY2g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
Subject: Re: [PHP-DEV] Consensus Gathering: is_initialized
From: a.leathley@gmx.net (Andreas Leathley)

On 26.05.21 12:13, Joe Watkins wrote:
> Not absolutely convinced that it's a good idea, I asked Nikita to review=
,
> and he's unconvinced also and suggested a discussion should be started.
>
> You can read both of our thoughts about it on the PR.
>
> What I'm looking for now, is reasons that we are wrong - good use cases
> that aren't born from code that is doing bad things ...
>
> Equally if you think this is really bad even if the reason has already b=
een
> mentioned, please make noise.

I like it because it is very clear and takes into account the current
scope. The alternatives are more messy:

- When using "isset" on class properties, it is not obvious if you are
checking for initialized state or for null, as it does both

- When using reflection, scope does not matter (and you have to
explicitely set a property to accessible for private properties)

While it is not common that I need to check if a property is
initialized, I do think there should be a straightforward way to do it.
Setting properties through a constructor is not the only way of
initializing parts of a class - some use cases were already brought up,
like caching or lazy loading. Also, is_initialized cannot be done in
userland, as passing an unitialized property to a function already leads
to an error, so every way of checking now is a workaround that cannot be
made clearer for the reader of the code. I would prefer it if it also
worked for untyped properties, just to be consistent.

Static analyzers would also have a much easier time understanding the
code compared to now. When using isset on a non-nullable property a
static analyzer would rightfully complain that the property is not
nullable, as it assumes you are checking for null, not for uninitialized
- isset in this case is abused to check for uninitialized, hiding the
actual intent. Not sure how good static analyzers are at understanding
the reflection code, but is_initialized would make things clearer for
both humans and analyzers.