Revisiting proposal for addition of `get_class_constants()`

Aaand I sent that too hastily...Specifically for question 2 around
ReflectionConstant, I just noticed
ReflectionClass::getReflectionConstants() exists...Scratch that
question!

Sorry!

Hey Nikita,

Thanks for the thoughts.

Could you please explain in more detail why we should duplicate existing 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 historical 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

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.

I've also used it in the past for generating exception messages, because
the base Exception class requires $code to be int, so given a value of
1011, I wanted to map back to a constant like "MyException::TYPE_FOO".

If the concern is that we will end up with too many of these as global
functions, might there be value in a "lightweight reflection" API, which
gave access to things like this without the full power or performance cost
of Reflection?

Regards,

Rowan Tommins
[IMSoP]

Hey Nikita,

Thanks for the thoughts.

Could you please explain in more detail why we should duplicate
existing 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)

I don't think this comparison makes a lot of sense. str_contains() etc were
very much about providing a more convenient way to perform an extremely
common operation. It makes sense to make common operations more concise.

get_class_constants() is on the complete other side of the spectrum: It
would be a very niche API. It's okay to have niche APIs (Reflection as a
whole is pretty niche), but there's no point in optimizing them for concise
calls.

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.

That's correct, but I think it's acceptable as long as we consider those to
be historical artifacts.

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.

This is a good point. I think the right way to address this would be to
expand the reflection API though. It's a bit odd that visibility-based
filtering exists here, but is not available in the reflection extension (it
can filter by public/protected/private, but not by "visible in this
scope"). Having methods like ReflectionMethod::isAccessible(?string $scope
= null): bool would be a good addition, I think.

I believe the existence of functions like get_class_methods() is a
historical 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?

I don't think there's a particularly strong reason to remove those
functions. They're there and they don't hurt anyone :) I think there's a
middle ground between "these must be removed" and "these must be extended
further".

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?

As you mention in your next mail, this already exists :)

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.

I think the per-request cache here is the right approach. Reflection isn't
so slow that hitting it once per request would be problematic. At the same
time, no matter how this is implemented, you will still want to have that
cache, because fetching the class constants is still going to be slower
than not fetching them, regardless of how you do it. Having this as a free
function may be two times faster than doing it via reflection, but not
doing it at all is #INF times faster :)

Regards,
Nikita

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.

Two reactions to this: One, reflection isn't a facility for novices,
so there's no need to cater to it. If novices end up needing such a
method frequently, something else is wrong other than how accessible
that method is.

Two, reflection being all about reflecting over objects and classes,
it seems reasonable to limit it to an OO API. Any existing global
functions in that arena should just be considered legacy.
ReflectionClass is not final, and I can see plenty of use cases for
subclassing it.

Hey Nikita,

Thanks for the thoughts.

Could you please explain in more detail why we should duplicate existing 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 historical 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