Last week, Greg Young wrote a blog post about State Pattern Misuse. In this post he talks about how ugly the State pattern can become if some operations are not allowed when the context object is in a particular state. The solution he proposes is to use a separate domain class for each state in the model so that it contributes to the Ubiquitous Language.
Just to be clear, I agree and value this approach when it makes sense in the domain model. However, I do not like to entirely give up on the use of the State pattern either. We can eliminate some of the friction by using a very simple approach, namely role interfaces.
To show you a simple example, I took the code of one of the samples on the DoFactory web site and put in some refactoring. Please take a look at the original code before reading any further.
Now instead of having abstract methods on the State base class, I created three separate role interfaces:
public interface ICanDeposit { void Deposit(Double amount); } public interface ICanWithdraw { void Withdraw(Double amount); } public interface ICanPayInterest { void PayInterest(); }
Both the SilverState and the GoldState class implement all these interfaces, but the one we’re particularly interested in is the RedState class because only deposits should be allowed for this state.
class RedState : State, ICanDeposit { private const Double UpperLimit = 0.0; public RedState(State state) { Balance = state.Balance; Account = state.Account; } public void Deposit(double amount) { Balance += amount; StateChangeCheck(); } private void StateChangeCheck() { if(Balance > UpperLimit) Account.State = new SilverState(this); } }
The RedState therefore only implements the ICanDeposit interface. Just for the record, the code of the State base class is now dramatically reduced.
abstract class State { public Account Account { get; protected set; } public Double Balance { get; protected set; } }
With this setup we can use these role interfaces in the Account class to determine whether a particular operation is allowed for the current state.
public void Withdraw(double amount) { // One can not always withdraw if(false == CanWithdraw()) { // Should throw an exception or at least a towel Console.WriteLine("No funds available for withdrawal!"); return; } var canWithdraw = (ICanWithdraw)State; canWithdraw.Withdraw(amount); ... } private Boolean CanWithdraw() { return State is ICanWithdraw; }
This way we’re able to eliminate all operations that don’t make sense for a particular state while still being able to determine all of its capabilities when needed.
Hope this helps.
Hey Jan – nice idea although I’m not a massive fan of the State Pattern this gives a nice way of keeping it clean.
Can you explain one thing to me though please? In your code above an “Account” has a “State” property and a “State” object has an “Account” property. Now I like recursion as much as the next guy but how deep does this particular rabbit hole go? 😉
I haven’t see all the code of course so I may just be missing something.
Great post,
Rob G
The abstract base class from the DoFactory code you reference is violating the Interface Segregation Principle. I think this ISP violation is the real issue you’ve solved – the State pattern wasn’t really at fault.
@Rob G The recursion is indeed not the best thing since sliced bread. The original GOF example shows this as well.
@Jeff Doolittle: I agree, but this seems to be how its mostly used. The original example (TCPState) in the GOF book has the same ISP violation as well.
@Jan – yeah, I can see what you mean. I can think of a few other examples I’ve seen where the state pattern implementation contains ISP violations. I think your approach certainly helps redeem the pattern.
I’ve got to admit, I’m quite fond of the classic state pattern implementation. Implementing all of those methods where you can’t do anything may be a pain, but you end up putting in explicit feedback about why they can’t be done.
The issue with approaches like this is that you need to do a lot of type-checking and casting throughout the code for each applicable interface. Interfaces like that are good for simplifying cases where you need to pass an IDepostable etc. to a given object/service though.
I hope I understand your usage of explicit interfaces in this case, but wouldn’t it be possible to change all the interfaces to TryDo____ methods? No checks for implementing interfaces, no need to throw exceptions, all to do: returning true after any positive processing or no processing at all. What do you think about this idea?
Btw. I do like Greg’s proposal, cause the unusual cases are the majority of cases in this state pattern example.
I think a bit of what I was posting about was missed.
My issue is not with the state pattern per se though it can get ugly. it is more so with people misusing the state pattern. In my example there would be nothing in common between the 3 versions 🙂 it would be throws laced through much of the implementations. Even if we went to role interfaces we would end up with tons of code checking things. Beyond that there isnt any real use case that would actually be using the polymorphism … I find it to be quite a trend that people like to make things polymorphic even if they dont actually use the polymorphism … why? There is a big downside to it in terms of coupling.
Greg
@Greg: In my follow-up post I talked about how to mitigate the checking part when using role interfaces. You can remove the polymorphism in the same way using a DynamicObject. As I said, I totally agree with your post and the misuse of any design pattern for that matter. Everything has it advantages and disadvantages and the downside is indeed coupling. But I think there are valid scenarios for using the State pattern as well.
false == CanWithdraw()what?