Does the state Pattern violate Liskov Substitution Principle?

Question

This image is taken from Applying Domain-Driven Design and Patterns: With Examples in C# and .NET

This is the class diagram for the State Pattern where a SalesOrder can have different states during its life time. Only certain transitions are allowed between the different states.

Now the OrderState class is an abstract class and all its methods are inherited to its subclasses. If we consider the subclass Cancelled which is a final state that doesn't allow any transitions to any other states, we'll have to override all its methods in this class to throw exceptions.

Now doesn't that violate Liskov's substitution principle since a sublcass shouldn't alter the behavior of the parent? Does changing the abstract class into an interface fixes this?
How can this be fixed?

Answer 1

This particular implementation, yes. If you make the states concrete classes rather than abstract implementors then you will get away from this.

However the state pattern you're referring to which is effectively a state machine design is in general something I disagree with from the way I've seen it grow. I think there is sufficient grounds to decry this as a violation of single responsibility principle because these state management patterns end up being central repositories for knowledge of the current state of many other parts of a system. This state management piece being centralized will more often than not require business rules relevant to many different parts of the system to reasonably coordinate them.

Imagine if all the parts of the system which care about state were in different services, different processes on different machines, a central status manager that details the status for each of those places effectively bottlenecks this whole distributed system and I think that bottleneck is a sign of a SRP violation as well as generally bad design.

In contrast, I would suggest one make objects more intelligent as in the Model object in the MVC pattern where the model knows how to handle itself, it doesn't need an outside orchestrator to manage it's internal workings or reason for it.

Even putting a state pattern like this inside of an object so it is only managing itself, it feels like you would be making that object too large. Workflows should be done through composition of various self-responsible objects I would say, rather than with a single orchestrated state which manages the flow of other objects, or the flow of intelligence within itself.

But at that point it's more art than engineering and so it is definitely subjective your approach to some of those things, that said the principles are a good guide and yes the implementation you list is an LSP violation, but could be corrected not to be. Just be very careful about the SRP when using any pattern of this nature and you're likely to be safe.

Answer 2

a sublcass shouldn't alter the behavior of the parent?

That's a common misinterpretation of LSP. A subclass can alter the behavior of the parent, as long as it remains true to the parent type.

There is a good long explanation on Wikipedia, suggesting the things that will breach LSP:

... there are a number of behavioral conditions that the subtype must meet. These are detailed in a terminology resembling that of design by contract methodology, leading to some restrictions on how contracts can interact with inheritance:

• Preconditions cannot be strengthened in a subtype.
• Postconditions cannot be weakened in a subtype.
• Invariants of the supertype must be preserved in a subtype.
• History constraint (the "history rule"). Objects are regarded as being modifiable only through their methods (encapsulation). Since subtypes may introduce methods that are not present in the supertype, the introduction of these methods may allow state changes in the subtype that are not permissible in the supertype. The history constraint prohibits this. It was the novel element introduced by Liskov and Wing. A violation of this constraint can be exemplified by defining a MutablePoint as a subtype of an ImmutablePoint. This is a violation of the history constraint, because in the history of the Immutable point, the state is always the same after creation, so it cannot include the history of a MutablePoint in general. Fields added to the subtype may however be safely modified because they are not observable through the supertype methods. One may derive a CircleWithFixedCenterButMutableRadius from ImmutablePoint without violating LSP.

Personally, I find it easier simply to remember this: If I'm looking at a parameter in a method that has type A, would someone passing a subtype B cause me any surprises? If they would then there is a breach of LSP.

Is throwing an Exception a surprise? Not really. It's something that can happen at any time, whether I'm calling the Ship method on OrderState or Granted or Shipped. So I have to account for it and it's not really a breach of LSP.

That said, I do think there are better ways to handle this situation. If I were writing this in C#, I would use interfaces and check for the implementation of an interface before calling the method. For example, if the current OrderState doesn't implement IShippable, don't call a Ship method on it.

But then I also wouldn't use the State pattern for this particular situation. The State pattern is much more appropriate to the state of an application than to the state of a domain object like this.

So, in a nutshell, this is a poorly contrived example of the State Pattern and not a particularly good way to handle the state of an order. But it arguably doesn't breach LSP. And the State pattern, in and of itself, certainly doesn't.

Answer 3

(This is written from the point of view of C#, so no checked exceptions.)

According to Wikipedia article about LSP, one of the conditions of LSP is:

No new exceptions should be thrown by methods of the subtype, except where those exceptions are themselves subtypes of exceptions thrown by the methods of the supertype.

How should you understand that? What exactly are “exceptions thrown by the methods of the supertype” when the methods of the supertype are abstract? I think those are the exceptions that are documented as the exceptions that can thrown by the methods of the supertype.

What this means is that if OrderState.Ship() is documented as something like “throws InvalidOperationException if this operations is not supported by the current state”, then I think this design does not break LSP. On the other hand, if the supertype methods are not documented this way, LSP is violated.

But this does not mean this is good design, you should not use exceptions for normal control flow, which this seems very close to. Also, in a real application, you would probably want to know whether an operation can be performed before you attempt to do it, for example to disable the “Ship” button in the UI.