Object-Oriented Design – Why Is Square Inheriting from Rectangle Problematic?

abstractioninheritanceobject-oriented-designsolid

If a Square is a type of Rectangle, than why can't a Square inherit from a Rectangle? Or why is it a bad design?

I have heard people say:

If you made Square derive from Rectangle, then a Square should be
usable anywhere you expect a rectangle

What is the problem here? And why would Square be usable anywhere you expect a rectangle? It would only be usable if we create the Square object, and if we override the SetWidth and SetHeight methods for Square than why would there be any issue?

If you had SetWidth and SetHeight methods on your Rectangle base class
and if your Rectangle reference pointed to a Square, then SetWidth and
SetHeight don't make sense because setting one would change the
other to match it. In this case Square fails the Liskov Substitution
Test with Rectangle and the abstraction of having Square inherit from
Rectangle is a bad one.

Can someone explain the above arguments? Again, if we over-ride SetWidth and SetHeight methods in Square, wouldn't it resolve this issue?

I have also heard/read:

The real issue is that we are not modeling rectangles, but rather
"reshapable rectangles" i.e., rectangles whose width or height can be
modified after creation (and we still consider it to be the same
object). If we look at the rectangle class in this way, it is clear
that a square is not a "reshapable rectangle", because a square cannot
be reshaped and still be a square (in general). Mathematically, we
don't see the problem because mutability doesn't even make sense in a
mathematical context

Here I believe "re-sizeable" is the correct term. Rectangles are "re-sizeable" and so are squares. Am I missing something in the above argument? A square can be re-sized like any rectangle.

Best Answer

Basically we want things to behave sensibly.

Consider the following problem:

I am given a group of rectangles and I want to increase their area by 10%. So what I do is I set the length of the rectangle to 1.1 times what it was before.

public void IncreaseRectangleSizeByTenPercent(IEnumerable<Rectangle> rectangles)
{
  foreach(var rectangle in rectangles)
  {
    rectangle.Length = rectangle.Length * 1.1;
  }
}

Now in this case, all of my rectangles now have their length increased by 10%, which will increase their area by 10%. Unfortunately, someone has actually passed me a mixture of squares and rectangles, and when the length of the rectangle was changed, so was the width.

My unit tests pass because I wrote all my unit tests to use a collection of rectangles. I now have introduced a subtle bug into my application which can go unnoticed for months.

Worse still, Jim from accounting sees my method and writes some other code which uses the fact that if he passes squares into my method, that he gets a very nice 21% increase in size. Jim is happy and nobody is any wiser.

Jim gets promoted for excellent work to a different division. Alfred joins the company as a junior. In his first bug report, Jill from Advertising has reported that passing squares to this method results in a 21% increase and wants the bug fixed. Alfred sees that Squares and Rectangles are used everywhere in the code and realises that breaking the inheritance chain is impossible. He also does not have access to Accounting's source code. So Alfred fixes the bug like this:

public void IncreaseRectangleSizeByTenPercent(IEnumerable<Rectangle> rectangles)
{
  foreach(var rectangle in rectangles)
  {
    if (typeof(rectangle) == Rectangle)
    {
      rectangle.Length = rectangle.Length * 1.1;
    }
    if (typeof(rectangle) == Square)
    {
      rectangle.Length = rectangle.Length * 1.04880884817;
    }
  }
}

Alfred is happy with his uber hacking skills and Jill signs off that the bug is fixed.

Next month nobody gets paid because Accounting was dependent on being able to pass squares to the IncreaseRectangleSizeByTenPercent method and getting an increase in area of 21%. The entire company goes into "priority 1 bugfix" mode to track down the source of the issue. They trace the problem to Alfred's fix. They know that they have to keep both Accounting and Advertising happy. So they fix the problem by identifying the user with the method call like so:

public void IncreaseRectangleSizeByTenPercent(IEnumerable<Rectangle> rectangles)
{
  IncreaseRectangleSizeByTenPercent(
    rectangles, 
    new User() { Department = Department.Accounting });
}

public void IncreaseRectangleSizeByTenPercent(IEnumerable<Rectangle> rectangles, User user)
{
  foreach(var rectangle in rectangles)
  {
    if (typeof(rectangle) == Rectangle || user.Department == Department.Accounting)
    {
      rectangle.Length = rectangle.Length * 1.1;
    }
    else if (typeof(rectangle) == Square)
    {
      rectangle.Length = rectangle.Length * 1.04880884817;
    }
  }
}

And so on and so forth.

This anecdote is based on real-world situations that face programmers daily. Violations of the Liskov Substitution principle can introduce very subtle bugs that only get picked up years after they're written, by which time fixing the violation will break a bunch of things and not fixing it will anger your biggest client.

There are two realistic ways of fixing this problem.

The first way is to make Rectangle immutable. If the user of Rectangle cannot change the Length and Width properties, this problem goes away. If you want a Rectangle with a different length and width, you create a new one. Squares can inherit from rectangles happily.

The second way is to break the inheritance chain between squares and rectangles. If a square is defined as having a single SideLength property and rectangles have a Length and Width property and there is no inheritance, it's impossible to accidentally break things by expecting a rectangle and getting a square. In C# terms, you could seal your rectangle class, which ensures that all Rectangles you ever get are actually Rectangles.

In this case, I like the "immutable objects" way of fixing the problem. The identity of a rectangle is its length and width. It makes sense that when you want to change the identity of an object, what you really want is a new object. If you lose an old customer and gain a new customer, you don't change the Customer.Id field from the old customer to the new one, you create a new Customer.

Violations of the Liskov Substitution principle are common in the real world, mostly because a lot of code out there is written by people who are incompetent/ under time pressure/ don't care/ make mistakes. It can and does lead to some very nasty problems. In most cases, you want to favour composition over inheritance instead.

Related Topic