Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110493
MIME-Version: 1.0
References: <CAD2aB0df7hObz8=AOGHJKAR3Jc2tdAU=y0i-oR_XfqHo-rG1fQ@mail.gmail.com>
 <CAF+90c8_8N5xKe3EGms1i_TbB7v3vPVSjr0tYapiZyxU+0diFA@mail.gmail.com>
In-Reply-To: <CAF+90c8_8N5xKe3EGms1i_TbB7v3vPVSjr0tYapiZyxU+0diFA@mail.gmail.com>
Date: Thu, 11 Jun 2020 23:31:58 -0500
Message-ID: <CAD2aB0eNcSLHp+-XPLySfQxb08E6xfcC9htAA3C_M3Qyz15OpA@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)

Hey Nikita,

Thanks for the thoughts.

> Could you please explain in more detail *why* we should duplicate existin=
g 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 histor=
ical artifact, because they were introduced before Reflection was a thing. =
Unless there is a strong reason to the contrary, I would prefer reflection =
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 obvious=
 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