PHP Strict Standards: Declaration of should be compatible

Alex picture Alex · Jan 13, 2014 · Viewed 14k times · Source

I have the following hierarchy of classes:

class O_Base {...}

class O extends O_Base {...}

abstract class A_Abstract {
    public function save(O_Base $obj) {...}
}

class A extends A_Abstract {
    public function save(O $obj) {
        echo 'save!';
    }
}

$o = new O;
$a = new A;

$a->save($o);

When I run this code I get the message:

Strict Standards: Declaration of A::save() should be compatible with A_Abstract::save(O_Base $obj) in .php on line 21

I know about E_STRICT error level but I can't find (and understand) the reason of that behavior. Can anybody help me?

Answer

Elias Van Ootegem picture Elias Van Ootegem · Jan 13, 2014

Your code is a clear violation of the Liskov Substitution principle. The abstract class requires an instance of O_Base to be passed to the save method, therefore all of the children of the A_Abstract should be defined in such a way that they can accept all instances of O_Base. Your child class implements a version of save that further restricts the API. see another example here

In your code, A breaches the contract which Abstract_A enforces/describes. Just like in real life, if you sign a contract, you have to agree on certain terms, and all be in agreement over the terminology you're going to use. That's why most contracts start by naming the parties, and then saying something along the lines of "Henceforth Mr X will be referred to as the employee".
These terms, like your abstract type hints are non-negociable, so that, further down the line you can't say: "Oh, well... what you call expenses, I call standard wages"
Ok, I'll stop using these half-arsed analogies, and just use a simple example to illustrate why what you're doing is, rightfully, not allowed.

Consider this:

abstract class Foo
{
    abstract public function save(Guaranteed $obj);
    //or worse still:
    final public function doStuff(Guaranteed $obj)
    {
        $obj->setSomething('to string');
        return $this->save($obj);//<====!!!!
    }
}

class FBar extends Foo
{
    public function save(Guaranteed $obj)
    {
        return $obj->setFine(true);
    }
}

class FBar2 extends Foo
{
    public function save(ChildOfGuaranteed $obj)
    {//FAIL: This method is required by Foo::doStuff to accept ALL instances of Guaranteed
    }
}

See here, in this case the perfectly valid abstract class is calling the save method with an instance of Guaranteed. If you were allowed to enforce a stricter type-hint in the children of this class, you could easily break this doStuff method. To help you protect yourself against these kinds of self-inflicted wounds, child classes should not be allowed to enforce stricter types on methods they inherit from their parent classes.
Also consider the scenario where we're looping over some instances and checking if they have this save method, based on these instances being an instanceof Foo:

$arg = new OtherChildOfGuaranteed;
$array = array(
    'fb'  => new FBar,
    'fb2' => new FBar2
);
foreach($array as $k => $class)
{
    if ($class instanceof Foo) $class->save($arg);
}

Now this will work fine if you just hint at Guaranteed in the method signature. But in the second case, we've made the type-hint a little too strict, and this code will result in a fatal error. Have fun debugging this in a more complex project...

PHP is very forgiving, if not too forgiving in most cases, but not here. Instead of having you scratching your head 'till your hear falls out, PHP very sensibly gives you a heads up, saying the implementation of your method breaches the contract, so you must fix this issue.

Now the quick workaround (and it is one in common use, too) would be this:

class FBar2 extends Foo
{
    /**
     * FBar2 save implementation requires instance of ChildOfGuaranteed
     *  Signature enforced by Foo
     * @return Fbar2
     * @throw InvalidArgumentException
     **/
    public function save(Guaranteed $obj)
    {
        if (!$obj instanceof ChildOfGuaranteed)
            throw new InvalidArgumentException(__METHOD__.' Expects instance of ChildOfGuaranteed, you passed '.get_class($obj));
        //do save here...
    }
}

So, you just leave the abstract type-hint as-is, but use the docblocks to document your code