Liskov substitution design principle adaptation

Question

Let's say I have an abstract class bird, and one of its functions is fly(int height).

I have numerous different bird classes, each with it's own different implementation of fly, and the function is used extensively across the application.

One day my boss comes and demands I add a duck, which does everything the other birds do except that it doesn't fly but rather swims in the application's pond.

Adding the duck as sub-type of bird violates Liskov substitution rule because when calling duck.fly we would either throw an exception, do nothing, or violate the correctness principle.

How would you go about introducing this change while keeping in mind the SOLID design principles?

Answer 1

I see 3 options for you:

1. Use Bird as an abstract base class for the common functionality and derive from it FlyingBird and AquaticBird.

2. Use object composition and visitor pattern as described by Zoran Horvat: https://vimeo.com/195774910 (which is worthwhile to watch in any case) - Although, it seems an overkill in the case at hand.

3. Use the solution as you have described it.

At the end, It's all about the balance. If you're expecting many different birds with different abilities to join in, then you should seriously consider option 2, otherwise depending on how you use the classes later choose between 1 and 3.

Update as per Gilads comments about Option 2 (Object Composition & Visitor Pattern)

The main thing here is that you can have one class Bird and attach Abilities to it such as Flying and Swimming and maybe later on you decide that you have other abilities or classification of some kind. You can do it either by creating different interfaces (IFlyable, ISwimmable) and have something like this:

public class SomeFlyingBird : BirdBase, IFlyable 
{
 ....
}

public class Duck : BirdBase, ISwimmable
{
 ....
}

And then use the visitor pattern to visit the specific actions.

Or, if you look for something more robust and elegant you could Create different classes per Ability and attach them to the Bird class:

public class Bird/Animal
{
     string name;
     List<Ability> abilities = new List<Ability>();

     public Bird (string name)
     {
         this.name = name;
     }

     public void Add (Ability ability)
     { 
          this.abilities.Add(ability);
     }
}

You could later have some static class to create the birds:

public static Bird CreateDuck ()
{
     var duck = new Bird("Donald");
     duck.Add(new SwimAbility());
     return duck;
}

The Ability and Swim can look like this:

public abstract class Ability
{
    public virtual void Accept(AbilityVisitor visitor) =>
        visitor.Visit(this);
}

public class SwimAbility : Ability
{
    public override void Accept(AbilityVisitor visitor)
    {
        base.Accept(visitor);
        visitor.Visit(this);
    }
}

I Have not attached the full code as it's not mine and it's way too long for this answer. I say again that this looks like an overkill for the case you described.

I strongly advise you to watch the video from Zoran Horvat and download and play with the code he provides: http://www.postsharp.net/blog/post/webinar-recording-object-composition

Answer 2

The issue you identify is due to the following incompatible features of your model.

1. All birds can fly
2. Ducks are birds
3. Ducks can't fly

You need to change one of these to make the statements consistent. Which to change will be both a question of the most appropriate design for your use, and the issues involved in making the changes.

You could address this by introducing classes to distinguish between flying and flightless birds. Maybe to make the change smaller your bird class continues to have fly and so Duck is not a bird but is a flightless bird. Or maybe you extend birds to create the flyingbirds class and move the fly method to there - both would involve changes to existing code.

Throwing exceptions is kind of cheating on point 3 - your duck still doesn't really fly, it just makes the problem a run-time one instead of a design-time one. It might be quick, but it's not a very safe approach and it requires calling code avoiding calling fly on birds that happen to be ducks.

Whether allowing fly to do nothing for ducks is OK depends on what your calling code expects fly to do in general - in effect you are fixing the inconsistency by changing the meaning of fly in point 1 to "All birds can be asked to fly (although some will not)". In that case fly really becomes flyIfyouCan which might be an indication of imperfect design, but may be the pragmatic solution - especially when adapting an existing design.

The fact that you suggest the do-nothing option might indicate a route to least upheaval because if you did let fly do nothing for ducks then you'd still need some duck-specific code to make it swim, so you could accept that in the real world ducks can fly, and have the duck-specific code call swim instead of fly - not as well as fly.

More generally I think what you're actually describing is a change of point 1 from "all birds can fly" to "all birds can move" and then non-ducks implement move as fly and ducks implement move as swim (whether they also have a fly method or not). This would probably involve changing some existing calls to fly into calls to move.

Answer 3

So your problem here is

(1) Duck is a Bird.

(2) But it can't fly.

So the real problem is,

• You have implemented your Bird class assuming "All birds can fly".
• But it is really not. So you either haven't thought of corner cases (like Duck) before-hand or haven't intended to have classes like Duck.

Solution:

So the simplest and wisest solution to be adopted here is use a generic name for the method name, so that both parties can agree. So here you can rename your method Bird.fly() into Bird.move().

So that both Duck.move() and Crow.move() makes sense, but they do in their own way.

All the other available solutions would kill Polymorphism (Using two different base classes). Which means you would not be able to call both Duck and Crow to move (swim and fly respectively) by one single method. (like Bird.move() do).

It will cost you a lot when your application builds on because you have to differentiate them all the time by explicit or implicit type casting, which is very bad. It will kill your time and make your application very hard to extend and maintain. :))