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 :)
Small syntax error in the first code example. Missing $ in front of parent::__construct(message
Otherwise a good post!
really backwards opinion, get an IDE.
@Christer: no $ is missing. "parent" is a keyword: http://php.net/manual/en/keyword.parent.php.
@Jonathan Chan: I guess he means before "message" ;)
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?
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 ?
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.
@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 :)
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.