ReflectionClass::getMethod()

php.internals

Andi Gutmans

21 years ago
Hi, It seems that in the past few months ReflectionClass::getMethod() was changed to throw an Exception if the method doesn't exist. I don't understand the reasoning because as it's a reflection API I would expect it to return false and not an exception. Exceptions should be thrown for errors. As long as we don't have hasMethod() then I think this behavior is wrong. Can anyone shed some light on this? Why was this changed? Marcus? Thanks, Andi

Timm Friebe

21 years ago
On Wed, 2004-12-22 at 14:28 -0800, Andi Gutmans wrote:
> Hi, > > It seems that in the past few months ReflectionClass::getMethod() was > changed to throw an Exception if the method doesn't exist.
-- snip -- revision 1.113 date: 2004/07/19 19:14:10; author: sebastian; state: Exp; lines: +9 -5 Make ReflectionClass::getMethod() and ReflectionClass::getProperty() raise an ReflectionException instead of returning NULL on failure. -- snip --
> I don't understand the reasoning because as it's a reflection API I > would expect it to return false
NULL would be better, IMO, but nevertheless.
> and not an exception. Exceptions should be thrown for errors. As long > as we don't have hasMethod() then I think this behavior is wrong.
Agreed.
> Can anyone shed some light on this? Why was this changed? Marcus?
Ask Sebastian:) I think what Sebastian wanted to do is: $reflectionClass->getMethod('abc')->invoke($object); ...and not have this bail with a fatal error when getMethod() returns NULL, but raise an exception. Fine. I still think NULL->method() should throw a NullPointerError instead of bailing but I also agree with Andi that as long as there is no hasMethod() throwing an exception is unacceptable.
-- Timm If it ain't broken, it doesn't have enough features yet

Marcus Börger

21 years ago
Hello Timm, Thursday, December 23, 2004, 1:06:01 AM, you wrote:
> On Wed, 2004-12-22 at 14:28 -0800, Andi Gutmans wrote: >> Hi, >> >> It seems that in the past few months ReflectionClass::getMethod() was >> changed to throw an Exception if the method doesn't exist.
> -- snip -- > revision 1.113 > date: 2004/07/19 19:14:10; author: sebastian; state: Exp; lines: +9 > -5 > Make ReflectionClass::getMethod() and ReflectionClass::getProperty() > raise an ReflectionException instead of returning NULL on failure. > -- snip --
>> I don't understand the reasoning because as it's a reflection API I >> would expect it to return false
> NULL would be better, IMO, but nevertheless.
>> and not an exception. Exceptions should be thrown for errors. As long >> as we don't have hasMethod() then I think this behavior is wrong.
> Agreed.
>> Can anyone shed some light on this? Why was this changed? Marcus?
> Ask Sebastian:)
> I think what Sebastian wanted to do is:
> $reflectionClass->getMethod('abc')->invoke($object);
> ...and not have this bail with a fatal error when getMethod() returns > NULL, but raise an exception. Fine. I still think NULL->method() should > throw a NullPointerError instead of bailing but I also agree with Andi > that as long as there is no hasMethod() throwing an exception is > unacceptable.
While i think using exceptions consistently everywhere in Reflection API is a good idea i also think we should have hasMethod() in either way. Best regards, Marcus mailto:helly@php.net

Andi Gutmans

21 years ago
Thanks Timm. Sebastian, what do you think? Andi At 01:06 AM 12/23/2004 +0100, Timm Friebe wrote:

Sebastian Bergmann

21 years ago
Andi Gutmans wrote:
> Exceptions should be thrown for errors.
And trying to get a method that does not exist is an error.
> Can anyone shed some light on this? Why was this changed?
To make consistent use of ReflectionException, IIRC. For instance, the following code <?php $method = new ReflectionMethod( 'NotExistingClass', 'notExistingMethod' ); ?> throws an ReflectionException as well. Now one could argue that there is no other way to signal an error in the __construct() than to use methods, but I think it is better to use the same error signalling in bith ReflectionClass::getMethod() and ReflectionMethod::__construct(). And besides, it is what Java does [1], so we cannot be that far off :-) Seasons's Greetings, Sebastian
-- [1] http://java.sun.com/j2se/1.5.0/docs/api/java/lang/Class.html -- Sebastian Bergmann http://www.sebastian-bergmann.de/ GnuPG Key: 0xB85B5D69 / 27A7 2B14 09E4 98CD 6277 0E5B 6867 C514 B85B 5D69

Sebastian Bergmann

21 years ago
Sebastian Bergmann wrote:
> methods, but I think it is better to use the same error signalling in
- methods, + exception,
-- Sebastian Bergmann http://www.sebastian-bergmann.de/ GnuPG Key: 0xB85B5D69 / 27A7 2B14 09E4 98CD 6277 0E5B 6867 C514 B85B 5D69

Timm Friebe

21 years ago
On Thu, 2004-12-23 at 07:43 +0100, Sebastian Bergmann wrote:
> Andi Gutmans wrote: > > Exceptions should be thrown for errors. > > And trying to get a method that does not exist is an error.
...which is fine, but without an elegant way of checking if that method exists I don't think it should be done. I think we could all agree on bool hasMethod(string $name) (and not changing getMethod()) though, right?
-- Timm If it ain't broken, it doesn't have enough features yet

Sebastian Bergmann

21 years ago
Timm Friebe wrote:
> > I think we could all agree on > > bool hasMethod(string $name) > > (and not changing getMethod()) though, right?
Yes, although I do not think that hasMethod() is really needed.
-- Sebastian Bergmann http://www.sebastian-bergmann.de/ GnuPG Key: 0xB85B5D69 / 27A7 2B14 09E4 98CD 6277 0E5B 6867 C514 B85B 5D69

Andi Gutmans

21 years ago
Adding hasMethod() sounds good to me. Sebastian, throwing an exception is relatively slow and cumbersome and sucks if you want to do something like a Delegation model and run through objects and check if a method can be called. Having exceptions thrown each time the object doesn't have the method is crazy. Andi At 11:02 AM 12/23/2004 +0100, Sebastian Bergmann wrote:

Sebastian Bergmann

21 years ago
Andi Gutmans wrote:
> Sebastian, throwing an exception is relatively slow and cumbersome and > sucks if you want to do something like a Delegation model and run > through objects and check if a method can be called. Having exceptions > thrown each time the object doesn't have the method is crazy.
I do not think so (that it is crazy), for what it's worth. Changing it back to return FALSE instead of throwing an extension would break existing applications (at least PHPUnit2).
-- Sebastian Bergmann http://www.sebastian-bergmann.de/ GnuPG Key: 0xB85B5D69 / 27A7 2B14 09E4 98CD 6277 0E5B 6867 C514 B85B 5D69

Derick Rethans

21 years ago
On Fri, 24 Dec 2004, Sebastian Bergmann wrote:
> Andi Gutmans wrote: > > Sebastian, throwing an exception is relatively slow and cumbersome and > > sucks if you want to do something like a Delegation model and run > > through objects and check if a method can be called. Having exceptions > > thrown each time the object doesn't have the method is crazy. > > I do not think so (that it is crazy), for what it's worth.
I think it doesn't make sense if there is no method to actually check if it's there. Now you start throwing exeptions without having a way to avoid it. THAT is crazy. Derick
-- Derick Rethans http://derickrethans.nl | http://ez.no | http://xdebug.org

Sebastian Bergmann

21 years ago
Derick Rethans wrote:
> THAT is crazy.
Throwing away consistency is crazy, too.
-- Sebastian Bergmann http://www.sebastian-bergmann.de/ GnuPG Key: 0xB85B5D69 / 27A7 2B14 09E4 98CD 6277 0E5B 6867 C514 B85B 5D69

Christian Schneider

21 years ago
Derick Rethans wrote:
> I think it doesn't make sense if there is no method to actually check if > it's there. Now you start throwing exeptions without having a way to > avoid it. THAT is crazy.
I assume that Sebastian would use something along the lines of try { $reflectionClass->getMethod("foo"); $hasmethod = true; } catch ($e) { $hasmethod = false; } to emulate hasMethod(). I agree with Andi and Derick that this is not the way an API should work. My main point is that 'basic' PHP APIs should avoid forcing users to use exceptions whenever possible. Converting traditional error handling to 'higher level' exceptions is easy: if (!$method = $reflectionClass->getMethod("foo")) throw new NoSuchMethodException; but the other direction is a) harder to read, b) requires knowledge about exceptions and c) slower. It also illustrates one of my pet peeves about exceptions: Nobody (not even the Java gods according to Sebastian's "it is what Java does [1], so we cannot be that far off") knows how to use exceptions properly (-:C - Chris

Sebastian Bergmann

21 years ago
Christian Schneider wrote:
> I assume that Sebastian would use something along the lines of
You assume wrong. My point is that you should not use getMethod() when you do not know whether or not the method exists. If you do not know whether or not the method exists use getMethods() in PHP 5.0 or hasMethod() in PHP 5.1 to check. Calling getMethod() for a method that does not exist is an error and should be consistently treated as such by raising an exception. This has nothing to do with correct use or abuse of exceptions. Merry Christmas, Sebastian
-- Sebastian Bergmann http://www.sebastian-bergmann.de/ GnuPG Key: 0xB85B5D69 / 27A7 2B14 09E4 98CD 6277 0E5B 6867 C514 B85B 5D69

Christian Schneider

21 years ago
Sebastian Bergmann wrote:
> You assume wrong. My point is that you should not use getMethod() when > you do not know whether or not the method exists.
Ok, let's look at the options then: 5.0 - getMethods() [sure, you could probably use clever tricks to make this shorter but the general clumsiness will remain]: foreach ($reflectionClass->getMethods() as $m) { if ($m['name'] == "foo") { $method = $m; break; } } 5.1 - hasMethod(): You were saying before that we don't need this and that's what I was basing my asumption on ;-) If we DO have hasMethod() then it is IMHO still unnecessarily complicated: if ($reflectionClass->hasMethod("foo")) $method = $reflectionClass->getMethod("foo"); Call me old-fashioned but I like the assignment-as-truth-value style where you combine the assignement and check into one statement. De gustribus non est disbutandum though :-)
> Calling getMethod() for a method that does not exist is an error and > should be consistently treated as such by raising an exception.
I disagree. The same could be said for getenv(). It's not part of a so-called OO API but it's the same operation: Return a key's value. It's half-way down the Python road where accessing undefined array indices causes exceptions. I like my PHP like my coffee: Simple and sweet :-) Let me look at the problem from another point of view: What would your error handling with exceptions look like? If you do things like $reflectionClass->getMethod("foo")->... then you have to catch your exception somewhere. Otherwise there is no difference to returning null as the program aborts. Now if you catch it in the same function you could as easily check for null. If on the other hand you're NOT catching it there your application has to know how to react to an unknown method. This can quickly lead to encapsulation problems. My point being that exceptions are very hard to handle properly and people disagree on how to do it (as seen in this thread) so they should be used sparingly and seldom be forced onto people. That's all I'm saying :-) Happy holidays, - Chris

ilya77@gmail.com

21 years ago
Why not just returning null when a method does not exist? Actually CALLING a non-existing method should be treated as an error... On Fri, 24 Dec 2004 18:47:26 +0100, Christian Schneider <cschneid@cschneid.com> wrote:

Sebastian Bergmann

21 years ago
Christian Schneider wrote:
>> Calling getMethod() for a method that does not exist is an error and >> should be consistently treated as such by raising an exception. > > I disagree. The same could be said for getenv().
The Reflection API was introduced at the same time as exceptions. It therefore makes perfect sense to make consistent use of exception in the Reflection API.
> My point being that exceptions are very hard to handle properly and > people disagree on how to do it (as seen in this thread) so they should > be used sparingly and seldom be forced onto people.
So the people who know how to do it right should do without because of the people who do not know how to it right? This is absurd.
-- Sebastian Bergmann http://www.sebastian-bergmann.de/ GnuPG Key: 0xB85B5D69 / 27A7 2B14 09E4 98CD 6277 0E5B 6867 C514 B85B 5D69

Marcus Börger

21 years ago
Hello Derick, in 5.0 the solution is to use getMethods() and look for it. Since that is not the best to do and not what we really want to i'll commit johannes' patch for hasMethod() later today. marcus Friday, December 24, 2004, 1:00:44 PM, you wrote:
> On Fri, 24 Dec 2004, Sebastian Bergmann wrote:
>> Andi Gutmans wrote: >> > Sebastian, throwing an exception is relatively slow and cumbersome and >> > sucks if you want to do something like a Delegation model and run >> > through objects and check if a method can be called. Having exceptions >> > thrown each time the object doesn't have the method is crazy. >> >> I do not think so (that it is crazy), for what it's worth.
> I think it doesn't make sense if there is no method to actually check if > it's there. Now you start throwing exeptions without having a way to > avoid it. THAT is crazy.
> Derick
> -- > Derick Rethans > http://derickrethans.nl | http://ez.no | http://xdebug.org
-- Best regards, Marcus mailto:helly@php.net