ramblings on PHP, SQL, the web, politics, ultimate frisbee and what else is on in my life
back 1  2  »  

Play nice when extending \Exception

This is just a short follow up to a recent tweet of mine. I have seen this repeatedly happen, even to top notch and usually very careful developers (*). I am not sure why this mistake happens so frequently, but quite often you see code that changes the parameter order for custom Exception constructors. I guess it's mostly because in these cases the developer wants to pass some magic parameters that contain the message (and potentially also the code). Another scenario is adding a new required parameter. In both cases don't! Thanks.


<?php
class FooException extends \Exception {
    private $mustBeSet;

    /**
     * Please never do this!
     */
    public function __construct($mustBeSet, $message, $code = null) {
        $this->mustBeSet = $mustBeSet;
        parent::__construct($message, $mustBeSet->getCode());
    }

}
?>

I guess if you really want to make the given parameter non optional, or do not want to have to pass a message and/or code, then use a factory method.


<?php
class FooException extends \Exception {
    static public function getInstance(Bar $bar) {
        return new self($bar->getMessage(), $bar->getCode());
    }
}
?>

Why do I care? I don't want to have to guess what the order is for standard stuff. Also its just a "OO no-no" since classes inheriting should not break the "is a" relation with their parents.

(*) Actually the above applies to any class and method really. I have been guilty of mucking with the signature of methods when using inheritance all the way back in my MDB/MDB2 days, where IIRC I was messing with the signatures when inheriting the PEAR_Error class. So not trying to be a smart ass, just sharing painful experiences :)

Update: As I stated in one of the comments I was not accurate when I said it violates the "is a" relation. I was trying to keep it simple and non academic, but while doing so essentially went overboard. So yes its not a violation per se. So I just consider it bad form :)

Comments



Re: Play nice when extending \Exception

Small syntax error in the first code example. Missing $ in front of parent::__construct(message

Otherwise a good post!

Re: Play nice when extending \Exception

really backwards opinion, get an IDE.

Re: Re: Play nice when extending \Exception

@Christer: no $ is missing. "parent" is a keyword: http://php.net/manual/en/keyword.parent.php.

Re: Play nice when extending \Exception

@Jonathan Chan: I guess he means before "message" ;)

Re: Play nice when extending \Exception

raveren: "really backwards opinion, get an IDE." How would you like it if your new car or bicycle would have the wiring changed so accelerating and braking would be the opposite of what you are used to?

Lukas: shouldn't your factory method be static?

Re: Play nice when extending \Exception

I Have to disagree....

Why should every exception in every project should have the same parameter order ?

I think it is legitimate to have other conventions, like SomeStuffException that accept a SomeStuff related informations as the first parameter.

Why should all my exceptions accept a message (The client code may not wan't to provide this information) and a "code" (I 've never used this information as the class of the exception itself is kind of a code) ?

Why do I care? I don't want to have to guess what the order is for standard stuff
You'll still have to guess what are the other parameters order..... and if code is not usefull at all you would have : new MyCustomException(null, null, $atLastSomeInformation) seriously ?

Re: Play nice when extending \Exception

I almost forgot :

And a Factory to solve the problem ?

So you're basically saying that : "I don't wan't to guess the parameters order..... but i'm ok to guess if it needs a Factory or not.... and then I'll be okay to guess the factory method parameters order."

I may have overlooked your point though.

Re: Play nice when extending \Exception

@Christer: thx fixed.

@Jan!: thx fixed.

@Oktopus: "guess" was probably the wrong word, "burned" is more like it. this is especially problematic if there isnt proper type checking on the modified signature. but like i explained with a factory you don't have to include a ton of default parameters at the start

One thing I should add is that above I was oversimplifying in that messing with the constructor signature is breaking the "is a relation". But I leave it to Toby to provide details there :)

Re: Play nice when extending \Exception

Although subclasses must respect the Liskov Substitution Principle, I never applied that to constructors as they are not part of the public interface of the class, at least the part interested by polimorphism. If you pass around an object, you can call its public methods, but not its constructor.

1  2  »