Just a quick run by posting from some discussions yesterday. I see many people using the State Pattern in their domains. More often than not though they are making a mistake that can be seen by analyzing the OO aspects of their code. Let’s go through a quick example of a Loan Application.
When its “In process” you can edit data on it or submit for approval.
When its “awaiting approval” you can no longer edit details data but you can approve or deny the application.
When its “completed” it has become immutable and can no longer be edited but may have some getters on it for viewing of historical data
I went through and made a quick little example of this
public interface ILoanApplicationState
{
void Approve();
void Deny();
InterestRate GetApprovedInterestRate();
InterestRate CalculateLikelyInterestRate();
void SubmitForApproval();
void ChangeDetails(LoanDetails details);
}
public class LoanApplicationState : ILoanApplicationState
{
private LoanApplication parent;
public LoanApplicationState(LoanApplication Parent)
{
parent = Parent;
}
public virtual void Approve() {}
public virtual void Deny(){}
public virtual InterestRate GetApprovedInterestRate() {}
public virtual InterestRate CalculateLikelyInterestRate() {}
public virtual void SubmitForApproval(){}
public virtual void ChangeDetails(LoanDetails details) {}
}
public class InProcessState : LoanApplicationState
{
public InProcessState(LoanApplication Parent) : base(Parent) { }
public override void Approve()
{
throw new InvalidOperationException("Cannot approve an inprocess application");
}
public override void Deny()
{
throw new InvalidOperationException("Cannot deny an inprocess application");
}
public override InterestRate GetApprovedInterestRate()
{
throw new InvalidOperationException("No approved interest rate exists on an in process application");
}
public override InterestRate CalculateLikelyInterestRate()
{
return InterestRate.Default;
}
public override void SubmitForApproval()
{
//change parent state to submitted
}
public override void ChangeDetails(LoanDetails details)
{
//change parent details
}
}
public class AwaitingApprovalState : LoanApplicationState
{
public InProcessState(LoanApplication Parent) : base(Parent) { }
public override void Approve()
{
//change parent state to approved and issue some behavior
}
public override void Deny()
{
//change parent state to denied and issue some behavior
}
public override InterestRate GetApprovedInterestRate()
{
throw new InvalidOperationException("No approved interest rate exists on an awaiting approval application");
}
public override InterestRate CalculateLikelyInterestRate()
{
return InterestRate.Default;
}
public override void SubmitForApproval()
{
throw new InvalidOperationException("Already awaiting approval");
}
public override void ChangeDetails(LoanDetails details)
{
throw new InvalidOperationException("No approved interest rate exists on an in process application");
}
}
public class CompletedApplicationState : LoanApplicationState
{
public InProcessState(LoanApplication Parent) : base(Parent) { }
public override void Approve()
{
throw new InvalidOperationException("Application has already been completed");
}
public override void Deny()
{
throw new InvalidOperationException("Application has already been completed");
}
public override InterestRate GetApprovedInterestRate()
{
//return parents approved interest rate
}
public override InterestRate CalculateLikelyInterestRate()
{
//return parents approved interest rate
}
public override void SubmitForApproval()
{
throw new InvalidOperationException("Already completed");
}
public override void ChangeDetails(LoanDetails details)
{
throw new InvalidOperationException("Application is already completed");
}
}
OK enough yucky code you guys get the idea (the state pattern is also very verbose). The idea here is we plug in these state objects to our object and they handle the behavior changes from the original object.
My contention here is that you should not use the State Pattern like this. Where is the polymorphism? What if we think about the gross violation of LSP that we just created? This does however happen way too often.
The solution is that we really have three classes here InProcessApplication, AwaitingApprovalApplication, and CompletedApplication.
public class InProcessApplication {
InProcessApplication SubmitForApproval(ILoanProcessingService processor) { … }
}
this will force us to end up with smaller and easier to understand classes… and I mean hey, its not like polymorphism would have worked before anyways. It also makes our Ubiquitous Language more concise.
There is a particular code smell here that will help us distinguish whether we want the State Pattern or separate classes. Do some of the data/behavior only make sense in certain states? If so use three separate classes. Going along with this, if we find “throws” in our state implementors we should realize through LSP that we are doing something bad.
Hi Greg,
Can you please be more elaborate and post a working sample ?
Apparently what I was describing in my last post comes closer to a PetriNet than a Finite State Machine is better modeled here
http://aabs.wordpress.com/2010/03/10/programming-with-petri-nets/
I’ve found that the traditional GoF pattern is limited and almost dangerous when used directly. I’ve implemented directly as you did above, and not been able to make heads or tails of my own code when I came back to it later. That’s very bad.
If you enhance it a bit by returning an IEnumerable
from your state operations, you get a lot of benefits:
- Encapsulate the transitions
- Increase readability: named transitions are easier to understand than the in-state code. Examples: CalculateInterestRate, Approve, etc
- Can reuse and compose the transitions
- Never have to throw unless you really want to .. and can still return error messages as needed. Think InvalidTransition(“ErrorMessage”)
- Clearly separate commands and queries: think CommandTransition, QueryTransition
- can add a command stack and undo functionality quite easily
I’ve found this to be very successful in maintaining states even across distributed, partially-connected systems. I just send the transitions around and if some fail, I roll back. Purely deterministic.
Thanks for the article Greg. I would also like to see the full solution if you get a chance to post it.
Greg,
Out of curiosity, can you give a scenario or two where you *would* use the state pattern then? Maybe it’s just the type of problem I’ve had to work on, but I can’t see many cases where you wouldn’t need to throw in some states.
Isn’t that the whole point of the pattern? Use state to help define the valid transitions between states – and if the invalid transitions don’t throw, what do they do?
Regards,
Remi.
“its not like polymorphism would have worked before __anyway__.”
There is no such word as ‘anyways’.
Hey Greg,
that’s exactly what I’ve been asked the other day for a job interview. I was asked how to implement a machine of throwing out a chewing gum each time I put a coin in. The traditional response was to implement the State pattern exactly as you mention it (WithCoin) (WithouCoin) and you are right, there are behaviour that doesn’t make sense at all for some state impl.
What does the method “throwOutChewingGum()” means in a “WithoutCoin” State? Obviously this perfect suites for throwing a runtime “NotImplementedYetException” other than business exception.
Well, in conclusion I gave roles to actors as ThrowOutMachine.throwOut(ThrowOutMachineContent, Money) where there’re State validations in the “ThrowOutMachine < -> ThrowOutMachineContent” collaboration that replaces the State pattern purpose.
So I ask myself given your post, is State pattern really a good solution? Should be first look for a collaboration pattern such as Container – Content
Yeah, ppl always tend to open the GoF book… I don’t know why, there’s always a better solution using tradicional OO.
Cheers,
Sebastian.
hi,
Great article.
Could you post the full code for the solution? I would like to compare the bad code with the good code as I understand the theory but would like to see how it is done practically.
Thanks,
Calum
Hey Greg,
that’s exactly what I’ve been asked the other day for a job interview. I was asked how to implement a machine of throwing out a chewing gum each time I put a coin in. The traditional response was to implement the State pattern exactly as you mention it (WithCoin) (WithouCoin) and you are right, there are behaviour that doesn’t make sense at all for some state impl.
What does the method “throwOutChewingGum()” means in a “WithoutCoin” State? Obviously this perfect suites for throwing a runtime “NotImplementedYetException” other than business exception.
Well, in conclusion I gave roles to actors as ThrowOutMachine.throwOut(ThrowOutMachineContent, Money) where there’re State validations in the “ThrowOutMachine < -> ThrowOutMachineContent” collaboration that replaces the State pattern purpose.
So I ask myself given your post, is State pattern really a good solution? Should be first look for a collaboration pattern such as Container – Content
Yeah, ppl always tend to open the GoF book… I don’t know why, there’s always a better solution using tradicional OO.
Cheers,
Sebastian.
So the Ubiquitous Language now speaks of 3 different Application classes as if they are separate? Conceptually, it’s always a single Application, with a unique Id, no? I think it might be clearer to think of it as a single stateful object.
I agree State is wrongly applied here because in this case, all the implementations that just throw an exception aren’t really valid operations, they should probably never be called.
In this case, I might rather just keep it to one class and do a check at the top of certain methods, throwing an exception if the method isn’t allowed.
That way, you could keep it to
1. retrieve the application by id
2. perform the operation
3. save the changes
If the UI was implemented correctly, those exception throwing operations should never even be called, right?
Canyou finish the example so that we can get the full taste of what you are saying
In the past I’ve handled this by adding properties such as CanApprove, CanDeny, etc. It changes the client usage pattern so that they must check if an action can be done before attempting to execute it. This takes care of the LSP violation because the client code should expect an exception when it calls a method whose corresponding Can* property is false. I have always gotten a bit of a code smell from this solution though. You make some good points here.
I’m glad you included the code in all its gory LSP-violating detail. But I’m not sure I see what you’re proposing as the alternative. Are the three classes derived from a common parent? If not, how are they consumed? What about common data for the application? Do you see an accompanying information holder for that stuff, e.g., LoanApplicationData?
“Do some of the data/behavior only make sense in certain states? If so use three separate classes. Going along with this, if we find “throws” in our state implementors we should realize through LSP that we are doing something bad. ”
Definitely agree it can be useful, but I do think its easily overused. Where there are a lot of states I’ve had success using separate classes each of which has 1 or more states.
On LSP, my view is its a LSP violation if a subclass breaks the contract by throwing an exception, if the exception is part of the contract of the base class then it isn’t.
Why not a LoanApplication base class in your final example?
Greg, I think you have a copy/paste issue with the constructors in your second and third state classes. Possibly another issue with this kind of pattern ;- )
Isn’t this about the “Tell, don’t ask” principle? If you have to call Approve() on something and get a response like “Already approved” it smells to me like asking the object for a state instead of telling the object to transition to another state.
I hope I get a chance to implement (or at least see) this pattern on some code soon. I’ll keep your post in my mind until then.
I think the state-pattern makes sense in order to implement state transition logic.
Perhaps the three classes even belong to different contexts…
Wouldn’t that be…
public class InProcessApplication {
AwaitingApprovalApplication SubmitForApproval(ILoanProcessingService processor) { … }
}
?