Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118372
Feedback-ID: i835c46d5:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-758-ge0d20a54e1-fm-20220729.001-ge0d20a54
Mime-Version: 1.0
Message-ID: <1404a189-f5ab-403f-b4a5-cc3789c6cc96@www.fastmail.com>
In-Reply-To: <6a5c06d3-3816-4d1b-8c9b-6884346812eb@www.fastmail.com>
References: <14237a5f-0522-48b4-8d3f-8dc346dd0870@www.fastmail.com>
 <6a5c06d3-3816-4d1b-8c9b-6884346812eb@www.fastmail.com>
Date: Sun, 07 Aug 2022 21:39:43 +0100
To: =?UTF-8?Q?Bj=C3=B6rn_Larsson?= <internals@lists.php.net>
Content-Type: multipart/alternative;
 boundary=72b7111962e4444cb2c671bef1d03572
Subject: Re: [PHP-DEV] RFC Idea: Introducing a ReflectionWithAttributes interface -
 Looking for feedback
From: php@ollie.codes ("Ollie Read")

--72b7111962e4444cb2c671bef1d03572
Content-Type: text/plain


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*

--72b7111962e4444cb2c671bef1d03572--