How should an object using the state pattern be transitioned to the next state?

Question

I have an Order class which goes through a series of defined states. To aid with this, I have implemented the State pattern such that the Order object has a CurrentState member which implements an IOrderState interface. I then have concrete implementations of this interface such as OrderStateNew, OrderStateDelivered etc etc

My question is, what is the correct way to transition the Order object between states? Is it acceptable to have an Order.SetState() method which allows an external service to set the state? The criteria which determine the state changes are stored externally to the Order object so this seems the obvious answer but I'm slightly uneasy about having a public method on my object to change something as fundamental as this.

Additional clarification I thought it might be useful to add some more detail about my implementation because I wonder if I'm actually using the pattern correctly in the first place. Here's the pulbic API for creating and authorising an order

Dim orderFacade As New OrderFacade
Dim order = orderFacade.createFrom(customer)

' Add lines etc

' This will validate the order and transition it to status 'Authorised'
Dim valid = orderFacade.Authorise(order)

' This will commit the order, but only if it is at status 'Authorised'
Dim result = orderFacade.Commit()

The OrderFacade.Authorise() function looks something like this

Public Function Authorise(ByRef originalOrder As Order) As ValidationSummary

    If originalOrder.CurrentState.CanAuthorise() Then

       Dim validator = OrderValidatorFactory.createFrom(originalOrder)
       Dim valid = validator.ValidateOrder(originalOrder)

      If valid.IsValid Then
          originalOrder.SetOrderStatus(OrderStatus.Authorised)
      End If

     Return valid

    End If

 End Function

As you can see, the CurrentState member is the current IOrderState implementation which determines which activities are valid for the object. I wonder whether this should be responsible for determining the transition rather than the OrderFacade?

Answer 1

You can reduce the visibility of the method to package private for example. but in your case I think this is the only way, another way its to have a parent abstract class that implements the state machine and just have a group of nextState(inputParameter) methods that will shift the state of the order to the corresponding state.

Answer 2

I don't think there's a "right" answer for this; it's really dependent upon the architecture you've chosen for your state machine. If all the logic for changing state were encapsulated within your Order class, then I would say it would be poor practice to expose a SetState method. However, since you have already placed some of the state-determining logic outside the Order class, it seems appropriate (and necessary) to expose a public SetState method, or something similar.

Of course, your other choice would be to move the state-determining logic into the Order class, although, based on what you've posted, it seems as if that would create a very complex class.

Anyway, in short, patterns are really just there to help you architect your code, not to set hard and fast rules. You should apply patterns to your architecture in the way that works best, not architect to patterns.

Answer 3

A SetState() method would have advantages in extending orders with further states, as well as instrumenting changes -- but I'd not recommend it. The State pattern is about collecting behaviours specific to distinct states in separate classes, not about how to present stateful interfaces to other classes.

For Orders, think about the business events which come naturally (e.g. Confirmation, Acknowledgement, Shipping Notice, Shipping, Invoice &c) and design an explicit interface around them. Exactly how you design the interface depends on how your application logic is structured, and how it is used from other layers. The classic answer is to define abstract methods for each business event (e.g. Confirm(), Acknowledge(), ShipDateChanged()). If you're using e.g. C# you might decide to go with incoming and outgoing events from your Order objects. Or you could try try some mixture or combination thereof. The main point is that a SetOrderState() interface isn't very descriptive, and is likely to lead to a clumsy implementation (large methods in each of your OrderState classes).

Then again, a SetState() method internal to your classes (called from each of your specific methods or events) could be OK if you don't have a lot of code around the different state changes: but I wouldn't expose that as an external interface. The drawback is that you may get some overlap between the methods of your internal IOrderState interface and the externally exposed Order interface.

This is a judgement call, but if I were you, I'd go with your instinct not to expose the details of your State implementation to clients. Code that uses your Order class should be readable and comprehensible.

Answer 4

I think, the transition between states should be in the class.

For Example, when you perform some kind of action in the order, the order knows how to move to other state. For Example:

 public void setPaid(int amount)
 {
    //Code to pay.
    this.State = new PaidState();   //State implements the IState Interface
 }

Other alternative could be to create a new Class for Example Transformer that implements a method like this.

 public class Transformer
 {
    public static void setState(Order o, IState s)
    {
         //Change the State.
    }
 }

Or yoy can use an Enum to set the States there:

 public class Transformer
 {
    public static void setState(Order o, StatesEnum s)
    {
         //Change the State.
    }
 }

But I recomend the class to manipulate its own state. But remember the complexity you will have on the other hand.

Best Regards!

Answer 5

Consider changing the state by implication rather than assignment.

In almost all cases I've ever seen, the State can be inferred from other properties (hopefully within the Class). If so, don't persist the state, but derive it when needed. Otherwise you'll often end up with problematic disagreements between inferred and assigned values. (And in my experience "derived" is invariably right.)

(The complexity often is to review the transaction log for the class, and consider the most recent event(s). But it's worth it anyway.)