Design Principles – Violation of the Liskov Substitution Principle?

designliskov-substitutionobject-oriented-designsolid

Say we have a list of Task entities, and a ProjectTask sub type. Tasks can be closed at any time, except ProjectTasks which cannot be closed once they have a status of Started. The UI should ensure the option to close a started ProjectTask is never available, but some safeguards are present in the domain:

public class Task
{
     public Status Status { get; set; }

     public virtual void Close()
     {
         Status = Status.Closed;
     }
}

public class ProjectTask : Task
{
     public override void Close()
     {
          if (Status == Status.Started) 
              throw new Exception("Cannot close a started Project Task");

          base.Close();
     }
}

Now when calling Close() on a Task, there is a chance the call will fail if it is a ProjectTask with the started status, when it wouldn't if it was a base Task. But this is the business requirements. It should fail. Can this be regarded as a violation of the Liskov substitution principle?

Best Answer

Yes, it is a violation of the LSP. Liskov Substitution Principle requires that

Preconditions cannot be strengthened in a subtype.
Postconditions cannot be weakened in a subtype.
Invariants of the supertype must be preserved in a subtype.
History constraint (the "history rule"). Objects are regarded as being modifiable only through their methods (encapsulation). Since subtypes may introduce methods that are not present in the supertype, the introduction of these methods may allow state changes in the subtype that are not permissible in the supertype. The history constraint prohibits this.

Your example breaks the first requirement by strengthening a precondition for calling the Close() method.

You can fix it by bringing the strengthened pre-condition to the top level of the inheritance hierarchy:

public class Task {
    public Status Status { get; set; }
    public virtual bool CanClose() {
        return true;
    }
    public virtual void Close() {
        Status = Status.Closed;
    }
}

By stipulating that a call of Close() is valid only in the state when CanClose() returns true you make the pre-condition apply to the Task as well as to the ProjectTask, fixing the LSP violation:

public class ProjectTask : Task {
    public override bool CanClose() {
        return Status != Status.Started;
    }
    public override void Close() {
        if (Status == Status.Started) 
            throw new Exception("Cannot close a started Project Task");
        base.Close();
    }
}

Related Solutions

Object-oriented – How to verify the Liskov substitution principle in an inheritance hierarchy

It's a lot more simple than that quote makes it sound, accurate as it is.

When you look at an inheritance hierarchy, imagine a method which receives an object of the base class. Now ask yourself, are there any assumptions that someone editing this method might make which would be invalid for that class.

For example originally seen on Uncle Bob's site (broken link removed):

public class Square : Rectangle
{
    public Square(double width) : base(width, width)
    {
    }

    public override double Width
    {
        set
        {
            base.Width = value;
            base.Height = value;
        }
        get
        {
            return base.Width;
        }
    }

    public override double Height
    {
        set
        {
            base.Width = value;
            base.Height = value;
        }
        get
        {
            return base.Height;
        }
    }
}

Seems fair enough, right? I've created a specialist kind of Rectangle called Square, which maintains that Width must equal Height at all times. A square is a rectangle, so it fits with OO principles, doesn't it?

But wait, what if someone now writes this method:

public void Enlarge(Rectangle rect, double factor)
{
    rect.Width *= factor;
    rect.Height *= factor;
}

Not cool. But there's no reason that the author of this method should have known there could be a potential problem.

Every time you derive one class from another, think about the base class and what people might assume about it (such as "it has a Width and a Height and they would both be independent"). Then think "do those assumptions remain valid in my subclass?" If not, rethink your design.

Design Patterns – Does This Decorator Implementation Violate the Liskov Substitution Principle?

A WordSorter is-a WordBank, so code that works with a WordBank should also work when a WordSorter is used instead of a WorkBank. On the other hand, a WordSorter is-not-a SomeWordBank. The compiler won't even let you use a WordSorter in place of a SomeWordBank, so the issue does not even begin to arise.

There might be a LSP violation, but doesn't appear to be from the minimal specification you've given. Does WordSorter guarantee, for example, that one can add arbitrary strings to it and retrieve them all in the same order later? Then sorting the words would indeed break the that contract, and code that works for "correct" WordBanks can be broken by substituting a WordSorter. Whether there is such a guarantee is impossible to tell from the minimal UML diagram you've shown. If, for example, WordBank's contract says all words which are added are included in the result of getWords, then:

bank.add(w);
AssertContains(bank.getWords(), w);

should always work. That code would break if bank was a WordSorter, and it's WordSorter's fault for breaking the contract and hence violating the LSP. But if WordBank offers no such guarantee, then the above code is in the wrong (in the same way asser x*y == 42 will usually fail) and WordSorter is a perfectly well-behaved subclass.

Best Answer

Related Solutions

Object-oriented – How to verify the Liskov substitution principle in an inheritance hierarchy

Design Patterns – Does This Decorator Implementation Violate the Liskov Substitution Principle?

Related Topic