Liskov Substitution Principle 1

[sheila11]

Hi all,

The article here shows a 'bad' example of code that breaks the LSP:

http://davidhayden.com/blog/dave/archive/2005/06/10/1226.aspx

The code is:
public class Rectangle
{
protected int _width;
protected int _height;

public int Width
{
get { return _width; }
}

public int Height
{
get { return _height; }
}

public virtual void SetWidth(int width)
{
_width = width;
}

public virtual void SetHeight(int height)
{
_height = height;
}
}

public class Square: Rectangle
{
public override void SetWidth(int width)
{
_width = width;
_height = width;
}

public override void SetHeight(int height)
{
_height = height;
_width = _height;
}
}

[TestFixture]
public class RectangleTests
{
[Test]
public void AreaOfRectangle()
{
Rectangle r = new Square();

r.Width = 5;
r.Height = 2;

// Will Fail - r is a square and sets
// width and height equal to each other.
Assert.IsEqual(r.Width * r.Height,10);
}
}

The author says:
Square should not be derived from Rectangle (at least not coded like this with these expectations anyway). But does not show the correct way of implementing Square.

Could anyone help me understand the correct implementation?

TIA
Sheila

 

[PGO01]

    r.Width = 5;
    r.Height = 2;

This would not compile, because Width and Height are read only properties. It should be:

    r.SetWidth(5);
    r.SetHeight(2);

However, as you say, this implementation could produce unexpected results.

I would have in the Rectangle class an overloaded virtual method to set the size. This should then be overridden in the Square class to add square-specific validation like so:

public class Rectangle
{
    private int _Width;
    public int Width { get { return _Width; } }

    private int _Height;
    public int Height { get { return _Height; } }

    public virtual void SetSize(Size newSize)
    {
        this.SetSize(newSize.Width, newSize.Height);
    }

    public virtual void SetSize(int newWidth, int newHeight)
    {
        this._Width = Width;
        this._Height = Height;
    }
}

public class Square : Rectangle
{
    public override void SetSize(int newWidth, int newHeight)
    {
        if (!newWidth.Equals(newHeight))
            throw new ArgumentException("A square's width and height must be equal.");

        base.SetSize(newWidth, newHeight);
    }
}

 

[jmeckley]

@PO001: while that works, I find the exception is sloppy and doesn't truly reveal the the intention. Shapes are a simple example, but I would approach it like this

inteface Shape
{
   int Width {get;}
   int Height {get;}
}

class Rectangle : Shape
{
   public Rectangle(int width, int height)
   {
        Width = width;
        Height = height;
   }

   public int Width {get; set;}
   public int Height {get; set;}
}

class Square : Shape
{
   public Square(int width)
   {
       Width = width;
       Height = height;
   }

   public int Width {get; private set;}
   public int Height {get; private set;}
}

Shape is the common denominator.

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[sheila11]

Thanks PG001 and jmeckley.

I think PG001's solution resolves the issue with LSP, while jmeckley's solution by-passes the issue by not having any method in base class.

My confusion over LSP is this:
With Polymorphism we are supposed to get the ability to achieve different results depending upon which object we call the method on: base or derived. But LSP objects to the very fact that the results are different. Isn't this a contradiction?

TIA,
Sheila

 

[jmeckley]

both of our solutions adhere to LSP. PGO01 solved this with an overload and exception. I solve this using ctor args. both square and rectangle are a shape, so I created a class (interface) for this concept. I could just as easily have square inherit from rectangle like this

class square : rectangle
{
   public square(int width) : base (width, width)
}

the driving force behind LSP is that you should not have down cast an object to subclass to preform an operation.
reviewing the original code you provided, i don't see anything wrong with it. it adheres to LSP in a clean manner. It may be the example is trite so it's difficult to spot the "ugliness".

here is an example of a LSP violation

abstract class Animal
{
}

class Cat : Animal
{
  public void Meow();
}

class Dog: Animal
{
  public void Bark();
}

var animals = new Animal[]{new Cat(), new Dog()};
foreach(var animal in animals)
{
   if(animal is Cat)
   {
       ((Cat)animal).Meow();
   }
   else if(animal is Dog)
   {
      ((Dog)animal).Bark();
   }
}

this violates LSP because we have to determine the type of animal and down cast before calling the member.
there are ways to solve this problem. one option is to introduce a member to Animal that represents the subclass members (for example Speak());
Another approach, remove the Animal superclass from Cat and Dog and create an Animal Adapter object (again with a common member on Animal)

interface Animal
{
   void Speak();
}
class Cat
{
   public void Meow();
}

class Dog
{
   public void Bark();
}
class CatIsAnAnimal : Animal
{
   Cat cat;
   CatIsAnAnimal(Cat cat)
   {
      this.cat = cat;
   }

   public void Speak()
   {
      cat.Meow();
   }
}
class DogIsAnAnimal : Animal
{
   Dog dog;
   DogIsAnAnimal(Dog dog)
   {
      this.dog= dog;
   }

   public void Speak()
   {
      dog.Bark();
   }
}

var animals = new Animal[]
                 {
                     new CatIsAnAnimal(new Cat()), 
                     new DogIsAnAnimal(new Dog())
                 };
foreach(var animal in animals)
{
   animal.Speak();
}

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[PGO01]

@jmeckley: you're right - removing the possibility of using the classes incorrectly is better than throwing an exception.

There is a problem with your code though (typo I think):

class Square : Shape
{
   public Square(int width)
   {
       Width = width;
       Height = height; // should be: Height = width;
   }

   public int Width {get; private set;}
   public int Height {get; private set;}
}