Re: RFC Idea: Introducing a ReflectionWithAttributes interface - Looking for feedback

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*