4

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?

enter image description here

  • I don't believe there is a change in the "behavior" of the program if there is only a change in the values offered. – BobDalgleish May 23 '14 at 21:33
  • @BobDalgleish Nearly everything interesting about a program's behavior revolves around the data it handles. Output routines need to be fed data, computations turn data into other data, control flow can depend heavily on data, and once you leave the cozy warmth of structured programming, even code itself (or at least behavior in the form of polymorphism) is data. When a List<int> starts ignoring the data put into it and instead always contains 17 copies of the number 42, every nontrivial program using List<int> goes haywire. – user7043 May 23 '14 at 21:35
  • 1
    @delnan You have completely ignored the point of both the question and LSP. WordSorter may safely be substituted for WordBank because there is no change in the behavior. – BobDalgleish May 23 '14 at 21:38
  • @BobDalgleish Read my answer for what I have to say on LSP and the question. My point towards you is that the data offered may or may not count as a change of behavior, depending on what constraints the supertype sets on the values offered. – user7043 May 23 '14 at 21:41
11

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.

  • Thanks for answering. I think that what I'm asking is this: Say you have a WordBank instance and a WordSorter instance. They are both fed the same array of strings. In this case, these two lines of code: wordBank.getWords() and wordSorter.getWords() - return different data. I guess that my question is: a WordSorter will return different data than a WordBank. The data will be of the same type, and the method signatures are the same - so no compile error will occur by substituting a WordBank with a WordSorter. But the data is different. Is this a violation of LSP? – Aviv Cohn May 23 '14 at 21:57
  • 4
    @Prog Again, it completely depends on the contract of WordBank. The sole fact that it returns different data (assuming WordBank wasn't abstract) is not a problem. It would be quite silly to mandate that subtypes are identical. They only must be compatible and what's compatible depends entirely on what you're doing. It's up to the person defining WordBank. If they say a WordBank returns the same words in the same order, it's an LSP violation. If they say a WordBank returns a subset of the word in arbitrary order, then it's fine. It's a weaker contract but a valid and useful one. – user7043 May 23 '14 at 22:07
  • I see. So it depends on what getWords() in WordBank was supposed to do in the first place. If it's meant to 'return all of the strings it contains', than there's an LSP violation. But if it's meant to 'return part of the strings it contains', then it's not an LSP violation since this contract is fulfilled both with WordBank and WordSorter. Is my understanding correct? – Aviv Cohn May 23 '14 at 22:13
  • @Prog Yes, that sounds good. I also added an example to the answer. – user7043 May 23 '14 at 22:15
  • And also: if the 'contract' of getWords() in WordBank is "do some or no sorting of strings, and return the resulting list of strings", but the client code was using getWords() while assuming it meant "always return all the strings", the code will break when subsituting with a WordSorter - but that's the client code's fault, not WordSorter's fault. Right? – Aviv Cohn May 23 '14 at 22:20
1

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).

  • Your last paragraph there might be the clearest explanation of the open/closed principle I've seen for a while. +1 for linking it to a good, concrete situation where it shows a real improvement in the code. – Jules Nov 18 '14 at 9:55
  • @Jules: One annoyance I have in Java and .NET is that the only construct for creating objects requires that the code invoking it specify the exact type to be created. While there are some situations where it's nice that new Foo() is guaranteed to return a reference to an instance of Foo to which no reference previously existed, there are many times when it would be nice to have an expression that makes clear its result type but is non-specific as to whether its exact type will be Foo or a derivative, or--for immutable types--whether the object will necessarily be new. – supercat Nov 18 '14 at 14:05
  • @Jules: IMHO, the normal pattern for object creation should be to call a factory method which then constructs an instance of its own type, but there's no way of knowing whether SomeType.someMethod() returns a type which has any relation whatsoever to SomeType. It would be nice if there were a means of invoking a static method which would make clear that the return type is the class upon which it is being invoked (and, in .NET, if interfaces could sport such static methods). – supercat Nov 18 '14 at 14:09
-2

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.....

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.