NDepend

Improve your .NET code quality with NDepend

SOLID design: The Liskov Substitution Principle (LSP)

The Liskov substitution principle is the L in the well known SOLID acronym. The original principle definition is:

Methods that use references to base classes must be able to use objects of derived classes without knowing it.

At first glance this principle is pretty easy to understand. At second glance it seems redundant with the OOP concept of polymorphism. After all, the whole point of polymorphism is to consume an abstraction without knowing the implementation behind isn’t it?

However it is a good thing that the community emphases the Liskov substitution principle. This principle is in fact a caveat for developers that polymorphism is both powerful and tricky : in the real world, the usage of polymorphism often leads to a dead-end situation, it must be wisely used.

LSP is often summarized with a counter-example of Duck Test“If it looks like a duck, quacks like a duck, but needs batteries – you probably have the wrong abstraction”

Let’s details some common polymorphism pitfalls that the Liskov substitution principle attempts to prevent by reminding the developer to adopt the client perspective.

Prevent situations where a method cannot be implemented

When a class implements an interface or derives from a base class, refactor tooling like Visual Studio refactor tools, Resharper or CodeRush propose to insert abstract methods stubs to implement. Typically the default body of such inserted method is throw NotImplementedException().

Obviously this behavior must remain temporary and must not be released in production. Client code that hold a reference on the interface or the base class doesn’t expect to get a NotImplementedException raised upon a method call. This is why NDepend has a default rule named Do implement methods that throw NotImplementedException, to prevent such situation.

On the other hand, if it doesn’t make sense to implement an abstract method, it clearly means that the design is wrong. Here is such wrong design, assuming that all birds can fly:

This code could then be refactored to:

The problem was that Bird with its Fly() method was too coarse, we needed some refinement because not all bird can fly. From my experience such wrong assumptions on interfaces and base classes happen quite often in the real world. When you stumble on such situation, see it as a good starting point for refactoring … if possible. Indeed, sometime refactoring is not an option if many clients depend already on the wrong design.

Example of a LSP violation in the .NET framework design

One dreaded LSP violation is .NET System.Array implementing the ICollection<T> interface. Hence Array has to implement the ICollection<T>.Add() method but calling this method on an array throws at runtime a NotSupportedException:

The C# compiler doesn’t even warn on such simple erroneous program.

Of course we’d need to ensure that ICollection<T>.IsReadOnly is false before modifying a collection through a reference of IList<T> or a ICollection<T> but frankly this is an error-prone design. I can remember having stumbled on this situation quite a few times during the last 15 years I am programming with C#/.NET.

Moreover refactoring this original design mistake is not an option anymore, even when .NET Core was introduced, since millions of programs are relying on this API. IReadOnlyCollection<T> has been introduced with .NET v4.5 in 2012 but the original design cannot be changed.

Think twice before applying the ISA trick

Another common example to explain the Liskov substitution principle is the Rectangle/Square paradigm. A square is-a rectangle isn’t it? So we should be able to write such code:

Clearly this is a wrong usage of the ISA principle: yes a square is a rectangle but the client doesn’t expect that when modifying the height of the rectangle, the width gets modified also.

Here also such wrong design emerges quite often in the real world. For example when dealing with a complex control hierarchy with dozens of fields to maintain at various level, it can become quite tricky to maintain coherence in your objects’ states at runtime.

Code Contract to help?

In theory code contract could help here. A contract is one or several assertions that prevent corrupted state. When a corrupted state is reached at runtime (whether in production or at test run time), such assertion must deadly fail because continuing running with corrupted state can potentially lead to more corrupted states.

Often contracts are used at interface level to improve the semantic by adding constraints to classes that implement the interface. We might want to force all implementations of IRectangle.set_Width to let the Height value untouched. By using Microsoft Code Contracts we could try to write something like that:

Unfortunately, as far as I know, Microsoft Code Contracts has no support for such PreserveValue() possibility. More generally it seems that Microsoft Code Contracts doesn’t receive much love nowadays despite how useful code contract can be. Ok C#8 non-nullable reference addresses many of the situations usually verified through contracts or assertions, but not all contracts are about nullable reference, as this example suggests.

Use polymorphism with great caution

These classical Bird and Rectangle examples show well how polymorphism and inheritance can quickly lead to rotten design, even in apparently simple situations. The Array class implementing ICollection<T> situation shows that in practice LSP violations just happen.

In my point of view, what the Liskov substitution principle really says is that when writing an API relying on polymorphism, you should first take the point of view of the client of your API before writing any interface and any class hierarchy.

  • Do really all birds can fly? What happen if I try to call Fly() on a bird that cannot fly?
  • Is a square really a rectangle? What happen if I change the width on a square?

In the real world this looks like:

  • Can all collections really be modified? What happen if I add or remove an element to an array?
  • Do all controls are scrollable? What happen if a scrollbar is displayed on a control that should not scroll?
  • Do withdrawal applies to all bank account? What happen if we try to withdraw money from a locked long term deposit account? Should we fail withdrawal badly in this situation or should we prevent such situation with an IAccountThatAuthorizeWithdraw abstraction?

A pattern emerges here: for each members of your interface you should question yourself: Do this member applies seamlessly to all objects that will implement this interface?

Notice that API writing principle is more general, it doesn’t only apply to polymorphism: when writing an API first take the point of view of the client of your API. This is the only way to achieve elegant API that clients will love to consume. This is another way to explain Test-Driven Development (TDD), where client code must be written for test and design purposes before writing the code itself.

In a previous post I explained why your classes should be declared as sealed when possible  and why NDepend proposes the default rule Class with no descendant should be sealed if possible. In the real world, a class is never well designed for inheritance by chance and thus should be sealed by default. Designing well a class for inheritance requires quite an extensive understanding of your domain. As for everything in life, it requires a lot of effort to build something that others will find easy to use.

 

 

My dad being an early programmer in the 70's, I have been fortunate to switch from playing with Lego, to program my own micro-games, when I was still a kid. Since then I never stop programming.

I graduated in Mathematics and Software engineering. After a decade of C++ programming and consultancy, I got interested in the brand new .NET platform in 2002. I had the chance to write the best-seller book (in French) on .NET and C#, published by O'Reilly (> 15.000 copies) and also did manage some academic and professional courses on the platform and C#.

Over the years, I gained a passion for understanding structure and evolution of large complex real-world applications, and for talking with talented developers behind it. As a consequence, I got interested in static code analysis and started the project NDepend.

Today, with more than 8.000 client companies, including many of the Fortune 500 ones, NDepend offers deeper insight and understanding about their code bases to a wide range of professional users around the world.

I live with my wife and our twin babies Léna and Paul, in the beautiful island of Mauritius in the Indian Ocean.

Published by

Patrick Smacchia

My dad being an early programmer in the 70's, I have been fortunate to switch from playing with Lego, to program my own micro-games, when I was still a kid. Since then I never stop programming. I graduated in Mathematics and Software engineering. After a decade of C++ programming and consultancy, I got interested in the brand new .NET platform in 2002. I had the chance to write the best-seller book (in French) on .NET and C#, published by O'Reilly (> 15.000 copies) and also did manage some academic and professional courses on the platform and C#. Over the years, I gained a passion for understanding structure and evolution of large complex real-world applications, and for talking with talented developers behind it. As a consequence, I got interested in static code analysis and started the project NDepend. Today, with more than 8.000 client companies, including many of the Fortune 500 ones, NDepend offers deeper insight and understanding about their code bases to a wide range of professional users around the world. I live with my wife and our twin babies Léna and Paul, in the beautiful island of Mauritius in the Indian Ocean.

Comments:

  1. Okay, suppose if I have an interface say INotify which has two methods sms and email. How should I use LSP is this case!

  2. Your interface violets the Single Responsibility Principle. You should create iNotify with one method notify() and implement it in two concrete classes – SmsSender and EmailSender

  3. Why not to implement ICollecttion.Add like this:

    void Add(T val)
    {
    Array.REsize(ref arr, arr.Length+1)
    arr[arr.Length-1]=val;
    }

  4. Why Rectangle has mutable properties with side effects in the first place? It rather should have a constructor accepting single value, the size of its sides. And if you will, a method Resize() that would return another Rectangle.

Leave a Reply

Your email address will not be published. Required fields are marked *