Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110494
MIME-Version: 1.0
References: <CAD2aB0df7hObz8=AOGHJKAR3Jc2tdAU=y0i-oR_XfqHo-rG1fQ@mail.gmail.com>
 <CAF+90c8_8N5xKe3EGms1i_TbB7v3vPVSjr0tYapiZyxU+0diFA@mail.gmail.com> <CAD2aB0eNcSLHp+-XPLySfQxb08E6xfcC9htAA3C_M3Qyz15OpA@mail.gmail.com>
In-Reply-To: <CAD2aB0eNcSLHp+-XPLySfQxb08E6xfcC9htAA3C_M3Qyz15OpA@mail.gmail.com>
Date: Fri, 12 Jun 2020 00:01:20 -0500
Message-ID: <CAD2aB0f6WwtqygJ48WLAHiyiHVisvnh0E+C283O0N=vOge3_iA@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Revisiting proposal for addition of `get_class_constants()`
From: troy.mccabe@gmail.com (Troy McCabe)

Aaand I sent that too hastily...Specifically for question 2 around
`ReflectionConstant`, I just noticed
`ReflectionClass::getReflectionConstants()` exists...Scratch that
question!

Sorry!

On Thu, Jun 11, 2020 at 11:31 PM Troy McCabe <troy.mccabe@gmail.com> wrote:
>
> Hey Nikita,
>
> Thanks for the thoughts.
>
> > Could you please explain in more detail *why* we should duplicate exist=
ing reflection functionality into free-standing functions?
>
> In terms of the *why*, there were three main reasons:
> 1. It aligns with the addition of the functions referenced in the
> original post (`str_[contains|starts_with|ends_with]()`), and one
> their stated reasons of simplifying the API for userland developers.
> While `(new \ReflectionClass(MyClass::class))->getConstants()` isn't
> the most difficult thing to grasp, it's not immediately clear to new
> developers, and is more verbose than
> `get_class_constants(MyClass::class)`
> 2. `get_class_[methods|vars]()` existing as built-in functions,
> creates a gap to retrieving class constants in the same way. If I
> start down the path of class inspection using `get_class_*()`, but
> find I can't retrieve constants in the same way, this is an
> inconsistency.
> 3. When using Reflection, accessibility is not respected as it is with
> the `get_class` family of functions. In the event that a developer is
> looking for constants which are accessible to the current context,
> there's no way (that I'm seeing, anyway) to retrieve _only_ constants
> accessible in the current context.
>
> > I believe the existence of functions like get_class_methods() is a hist=
orical artifact, because they were introduced before Reflection was a thing=
. Unless there is a strong reason to the contrary, I would prefer reflectio=
n functionality to stay inside Reflection...
>
> This is good background that I wasn't aware of (I knew the Reflection
> API was newer than the built-in functions, but not that the
> `get_class_*` functions were generally frowned upon).
>
> It does bring up 2 questions:
> 1. Obviously this is a much larger discussion, but is there any
> appetite to deprecate & remove the existing functions in favor of the
> Reflection API?
> 2. An alternative to adding `get_class_constants()` would be to
> introduce `ReflectionConstant` as a return type from
> `ReflectionClass::getConstants` to match `ReflectionMethod` &
> `ReflectionProperty`, which would solve point 3 above. Would this be a
> preferable approach?
>
> > You do mention performance as a benefit, but it's not immediately obvio=
us to me which use-cases are bottlenecked by class constant reflection.
>
> Enum implementations are the big case for this. While the libs I've
> looked at use an internal cache, these caches are per-request, so
> reflection will need to be used as many times as there are enums in a
> given system. Depending on the scale, this could be an appreciable
> amount. Obviously external caches could be leveraged, but that then
> requires additional development lift, instead of using battle-tested
> enum libs.
>
> Thanks for the thoughts, and thank you for all your work on internals!
> Thanks!
> Troy McCabe