Design – Violation of Liskov Substitution Principle Explained

designobject-oriented

I've have an animal class

class Animal
{
    public function eat(Food $food);
}

the subclass who inherit it actually cannot support all kinds of Food (Cat can only eat meat):

class Cat extends Animal
{
    public function eat(Food $food)
    {
        if (!$food instanceof Meat) throw new InvalidArgumentException();
    }
}

of course, Meat is a subclass of Food

So is this code violate LSP (I think it does)? and how to re-design it?

====================================

PS. The description above is an edited version. the original version is like below:

I've defined a data transformer interface

interface TransformerInterface
{
    /**
     * @return mixed
     */
    public function transform($origin); 
}

As you could see, $origin and the return type could be any type of data (I use PHP), however, the class who implements it actually cannot support all kinds of data, (I think it should be OK if it returns certain type of data, it doesn't violate LSP):

class TagTransformer implements TransformerInterface
{
    public function transform($origin)
    {
        if (!is_string($origin)) throw new InvalidArgumentException();
        ...
    }
}

So is this code violate LSP (I think it does)? and how to re-design it?

Best Answer

The LSP:

Let φ(x) be a property provable about objects x of type T. Then φ(y) should be true for objects y of type S where S is a subtype of T.

What is provable about Animal? You don't provide code or return type, presumably something like:

(Animal.Hungry = false AND Food.State = Consumed) OR InvalidArgumentException thrown

Is it still true for Cat? Yes (assuming you add the state changes).

Now what about Food and Meat? You don't prove any code, so we can assume the only difference is the Type. No violation is possible.

What about that type checking statement in Cat though? This looks bad, it's not a LSP violation, but it does raise questions about whether Food has a violation in it. If it doesn't, then I shouldn't need to check the Type right?

Really, the code smell is pointing out that Food should have some sort of FoodType property or CanBeEatenBy(Animal animal) method, rather than using SubTypes to identify the type.

Edit: it seems the correct design is in scope...

I'm going to use C#, which has a "flaw" in that you can't specify what exceptions might be thrown in the class definition. That being the case, and respecting the normal rule about not using exceptions for logic, I will try to avoid all exceptions.

Obviously, this isn't the only way to do this, it's just a way to avoid your code smell and problems with exceptions.

public enum AnimalType
{
    Omnivore,
    Herbivore,
    Carnivore
}

public class Food
{
    public bool IsEdibleBy(Animal a)
    {
        return true;
    }
}

public class EatenFood
{
    public override bool IsEdibleBy(Animal a)
    {
        return false;
    }
}

public class Animal
{
    public readonly AnimalType Type = AnimalType.Omnivore;
    public Food Eat(Food f)
    {
         if(f.IsEdibleBy(this))
         {
              return new EatenFood(f);
         }
         else
         {
              return f;
         }
    }
}

What is provable?

Eat always returns a type of Food 
IsEdibleBy always returns true or false
AnimalType is one of the enum values

Now we add Cat:

public class Cat : Animal
{
    public Cat() { this.Type = AnimalType.Carnivore; }
}

Public class Meat : Food
{
   public override bool IsEdibleBy(Animal a)
   {
       return a.Type == AnimalType.Omnivore || 
              a.Type == AnimalType.Carnivore;
   }
}

Now you have avoided all exceptions and LSP questions, you can extend your food types indefinitely, although you may have issues if you add more AnimalType enums. It would be important to make sure that that list is complete at the start.

Related Topic