Does code comply with Liskov's substitution principle?

Question

I'm testing my existing code on PHP5.4 prior to upgrade. I've discovered that the following code no longer works because PHP has tightened up its inheritance model. Due to this tightening, I've been reading about SOLID, and specifically Liskov's substitution principle (I'm a self taught programmer) so that I can improve my code and not suffer from future "tightenings".

interface IComparable {
    public function equals(self $other);
}

class A implements IComparable{
    protected $var;

    public function __construct($v){
        $this->var=$v;
    }

    public function equals(self $other){
        return ($this->var == $other->var) ? 'equal' : 'different';
    }
}

$a1= new A(7);
$a2= new A(5);
$a3= new A(5);

echo $a1->equals($a2),"\n";
echo $a2->equals($a3),"\n";

php 5.3 result:

• different
• equal

php 5.4 result:

PHP Fatal error: Declaration of A::equals() must be compatible with IComparable::equals(IComparable $other)

I can avoid the php5.4 error if I write the code this way:

interface IComparable {
    public function equals($other);
}

class A implements IComparable{
    protected $var;

    public function __construct($v){
        $this->var=$v;
    }

    public function equals($other){
        if(get_class($other) != get_class($this)) return false;
        return ($this->var == $other->var) ? 'equal' : 'different';
    }
}

but does the fix comply with Liskov's substitution principle since the function obviously won't accept any argument type? And if it doesn't, how can I code an inheritable function that will do what I need --compare 2 objects of the same type-- and comply with good OOD principles?

Answer 1

First of all: the PHP 5.3 behavior is a bug, so you can't use it as a yardstick to judge any other approach.

Going forward, the LSP was already violated in your 5.3 version code. Consider:

interface IComparable {
    public function equals(self $other);
}

This says that "any IComparable can compare itself to any other IComparable" (the semantics of the comparison are not important for the discussion). The class A then proceeds to violate the LSP because it does not support comparison with any IComparable -- just with those that happen to be A instances.

The PHP 5.4 version does not violate the LSP because the new version of IComparable says "I can compare myself to any other object" and that's exactly what class A does as well.

If your intent was to keep the 5.3 version contract, then IComparable should read

interface IComparable {
    public function equals(IComparable $other);
}

and class A would of course use the same signature. This would not violate the LSP and it would work correctly across both versions.

If your intent was to declare "an IComparable instance can compare itself with instances of the same type, whatever that is", then you are not in luck because this contract cannot be expressed using method signatures and type hints.

Update: Turns out your intent was to declare "instances of this class can compare themselves" and IComparable was intended simply to force you not to forget to do this.

In this case, the solution would be simply to forget about IComparable and use self in the signature of A::compare(). You do lose the compiler's forcing you to remember to define the requisite method, but that's IMHO a minor issue (especially because the interface only declares one method).

Answer 2

I believe your interface and class need to be written as follows to be more robust:

interface IComparable
{
    /** 
     *  @return boolean 
     */
    public function equals(IComparable $other);

    /**
     *  @return mixed
     */
    public function getValue();
}


class A implements IComparable{
    protected $var;

    public function __construct($v){
        $this->var=$v;
    }

    /**
     *  @return mixed
     */
    public function getValue()
    {
       return $this->var;
    }

    /** 
     *  @param  IComparable $other
     *  @return boolean 
     */
    public function equals(IComparable $other)
    {
        return ( $this->getValue() === $other->getValue() );
    }
}

The self keyword in php refers to the current class and I would advise against using it for type hinting arguments as it make's less clear what a method argument is.

With regards to your implementation of equals and Liskov Substitution this is an interesting subject you touch upon. I would see here for an interesting discussion about equality and Liskov.