On Sun, Aug 7, 2022, at 7:56 PM, Larry Garfield wrote:
> So I'm all for adding a proper interface here.  However, I am unclear why it 
> needs to be separate from Reflector.  Reflector is already used by all the 
> same classes that can have attributes, no?  Wouldn't just adding the 
> attribute methods to that interface be sufficient?

Sadly the Reflector interface is implemented by ReflectionExtension, which does 
not support attributes. It's also because adding those methods to Reflector now 
prevents any future implementation of that method for something that doesn't 
have attributes. Keep it simple, keep it separate.

> I'm mixed on the new methods.  The current equivalent of getAttribute() is:
> 
> $r->getAttributes($attrClass, \ReflectionAttribute::IS_INSTANCEOF)[0] ?? null;
> 
> Which I don't mind.  My main concern there would be how it interacts with 
> repeating/non-repeating attributes.  Can you call getAttribute() on a 
> repeating attribute?  Can you call getAttributes() on a non-repeating? Do 
> they behave any differently?  There's likely answers to these questions, but 
> they would need to be asked.

The methods would behave the same in terms of how it retrieves and returns the 
results, with the exception that getAttribute will only return one attribute or 
null. The main idea I have is that calling getAttribute for an attribute that 
is present more than once should throw an exception. 

> Would hasAttribute() and getNumberOfAttributes() have any performance benefit 
> over calling count() on getAttributes()?  Again, this isn't a pain point I've 
> had with attributes yet.  If so, I'd combine them into an attributeCount() 
> method that returns an int, and if it returns 0 you know it wasn't defined.

I'm afraid I couldn't say for certain, though it's highly likely that any 
performance increase would be negligible and not worth worrying about. I 
appreciate that they are easy to do in userland, but the same could be said for 
ReflectionFunctionAbstract::getNumberOfParameters and 
ReflectionClass::hasMethod.

The idea behind these two is to provide a nice API with sensible actions, while 
trying to maintain consistency, something we sorely need in the core.

I think the cost of having these methods is nothing compared to the nicer 
interface and API provided by having them as part of an interface.

---
Best Regards,
*Ollie Read*

Reply via email to