Do special-cases with fallbacks violate the Liskov Substitution Principle?

Question

Let's say I have an interface FooInterface that has the following signature:

interface FooInterface {
    public function doSomething(SomethingInterface something);
}

And a concrete class ConcreteFoo that implements that interface:

class ConcreteFoo implements FooInterface {

    public function doSomething(SomethingInterface something) {
    }

}

I'd like to ConcreteFoo::doSomething() to do something unique if it's passed a special type of SomethingInterface object (let's say it's called SpecialSomething).

It's definitely a LSP violation if I strengthen the preconditions of the method or throw a new exception, but would it still be an LSP violation if I special-cased SpecialSomething objects while providing a fallback for generic SomethingInterface objects? Something like:

class ConcreteFoo implements FooInterface {

    public function doSomething(SomethingInterface something) {
        if (something instanceof SpecialSomething) {
            // Do SpecialSomething magic
        }
        else {
            // Do generic SomethingInterface magic
        }
    }

}

Answer 1

It might be a violation of LSP depending on what else is in the contract for the doSomething method. But it's almost certainly a code smell even if it doesn't violate LSP.

For example, if part of the contract of doSomething is that it will call something.commitUpdates() at least once before returning, and for the special case it calls commitSpecialUpdates() instead, then that is a violation of LSP. Even if SpecialSomething's commitSpecialUpdates() method was consciously designed to do all of the same stuff as commitUpdates(), that's just preemptively hacking around the LSP violation, and is exactly the sort of hackery one would not have to do if one followed LSP consistently. Whether anything like this applies to your case is something you'll have to figure out by checking your contract for that method (whether explicit or implicit).

The reason this is a code smell is because checking the concrete type of one of your arguments misses the point of defining an interface/abstract type for it in the first place, and because in principle you can no longer guarantee the method even works (imagine if someone writes a subclass of SpecialSomething with the assumption that commitUpdates() will get called). First of all, try to make these special updates work within the existing SomethingInterface; that's the best possible outcome. If you're really sure you can't do that, then you need to update the interface. If you don't control the interface, then you may need to consider writing your own interface that does do what you want. If you can't even come up with an interface that works for all of them, maybe you should scrap the interface entirely and have multiple methods taking different concrete types, or maybe an even bigger refactor is in order. We'd have to know more about the magic you've commented out to tell which of these is appropriate.

Answer 2

It's not a violation of the LSP. However, it's still a violation of the "don't do type checks" rule. It would be better to design the code in such a way that the special arises naturally; maybe SomethingInterface needs another member that could accomplish this, or maybe you need to inject an abstract factory somewhere.

However, this isn't a hard and fast rule, so you need to decide whether the trade-off is worth it. Right now you have a code smell, and a possible obstacle to future enhancements. Getting rid of it might mean a considerably more convoluted architecture. Without more information I can't tell which one is better.

Answer 3

No, taking advantage of the fact that a given argument doesn't just provide the interface A, but also A2, does not violate the LSP.

Just make sure that the special path does not have any stronger pre-conditions (aside from those tested in deciding to take it), nor any weaker post-conditions.

C++ templates often do so to provide better performance, for example by requiring InputIterators, but giving additional guarantees if called with RandomAccessIterators.

If you have to decide at runtime instead (for example using dynamic casting), beware of the decision which path to take consuming all your potential gains or even more.

Taking advantage of the special case often goes against DRY (don't repeat yourself), as you might have to duplicate code, and against KISS (Keep it Simple), as it's more complex.

Answer 4

There is a trade-off between the principle of "don't do type checks" and "segregate your interfaces". If many classes provide a workable but possibly-inefficient means of performing some tasks, and a few of those can also offer a better means, and there is a need for code which can accept any of the broader category of items that can perform the task (perhaps inefficiently) but then perform the task as efficiently as possible, it will be necessary to either have all objects implement an interface that includes a member to say whether the more efficient method is supported and another to use it if it is, or else have the code which receives the object check whether it supports an extended interface and cast it if so.

Personally, I favor the former approach, though I wish object-oriented frameworks like .NET would allow interfaces to specify default methods (making larger interfaces less painful to work with). If the common interface includes optional methods, then a single wrapper class can handle objects with many different combinations of abilities while promising for consumers only those abilities present in the original wrapped object. If many functions are split into different interfaces, then a different wrapper object will be needed for every different combination of interfaces that wrapped objects might need to support.

Answer 5

Liskov substitution principle is about subtypes acting in accordance with a contract of their supertype. So, as Ixrec wrote, there is not enough information to answer whether it's an LSP violation.

What is violated here though is an Open closed principle. If you have a new requirement -- Do SpecialSomething magic -- and you have to modify existing code, then you're definitely violating OCP.