Does this Decorator implementation violate the Liskov Substitution Principle?

Question

Please consider the following implementation of the Decorator design pattern:

WordBank objects store strings and return them to the client through the method getWords().

The decorator class, WordSorter, is a subclass of WordBank (as in the Decorator pattern). However it's implementation of getWords() sorts some of the strings and removes them, before returning the array to the client.

For example a WordSorter might delete all of the strings in the bank that start with letter 'a', and only after that return the array to the client.

Does this violate the Liskov Substitution Principle? Since some implementations of WordBank return strings while others first sort through the strings and only return some of them, I'm not sure if it's safe to say that a WordSorter can be used anywhere any other WordBank is used. Or am I understanding this principle wrong?

Answer 1

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.

Answer 2

By the general description given, it sounds as though the concrete types WordBank and WordSorter (might be better called WordFilter) should be separate types both implement from a common interface IWordSource, and/or inherit from a common abstract base class WordSourceBase which does so, rather than having either concrete type inherit from the other (setting aside the question of what it should mean when a method returns an array). Code which wants to be able to store things into a word holder and know that items will be returned in the order supplied should demand a WordBank. Code that wants something that can supply a list of words, but doesn't care how that list is produced should demand an IWordSource. Code that wants something that can be given to code that expects an IWordSource but filters the supplied data first should create a WordSorter/WordFilter.

While I would not go so far as to suggest that instantiable classes should always be sealed, I would suggest that many situations where one might consider inheriting from an instantiable class would be better handled by having the original instantiable class inherit from an abstract base, and having the proposed derivative type also inherit from that same base. Code which is concerned about precise details of how the original instantiable class works (which might be different in the derived type) can demand a reference of that type, while code which is only interested in the abstract behaviors can accept a reference of that base type (which could then identify either the original type or a derivative).

Answer 3

Yes. Liskov is angry. But we can survive. The larger impact won't be felt until a couple more levels of magnitude of complexity are built upon these patterns. And even then it will be measured in lost time and efficiency. It will be felt in the negative space of what we can do with our minds given limited time upon this earth. In other words, nobody is gonna call you on it.....