I have some doubts about the following implementation of the state pattern:
I have an Order object. For simplicity, let's suppose it has a quantity, productId, price and supplier. Also, there are a set of known states in which the order can transition:
Order.isValid() changes between states. Ie, in state a some operations cannot be done. So, they look like:
void setQuantity(int q) {
if (_state.canChangeQuantity()) this.quantity = q;
else throw an exception.
}
Is this right, or should i get each state to implement setQuantity operation? In that case, where will be stored the value? In the order, or the state? In the latter case, i will have to copy the data in each state transition?
orderProcessor.process(order) is an object that checks order.IsValid, transition the order to some state, saves it to the db and perform some custom actions (in some states, admin is notified, in others the client, etc). I have one for each state.
In StateAOrderProcessor, the person that checks the order is notified via email, and the order is transitioned to state b.
Now, this pushes the state transitions outside the Order class. That means that Order has a 'setState' method, so each processor can change it. This thing to change the state from the outside does not sounds nice. Is that right?
Another option is to move all the validation logic to each state's processor, but now i have to track when a order's quantity was changed to see if that operation is valid in the current state. That leaves the order kind of anemic to me.
What do you think guys? Can you give me some advice to design this thing better?
This is an ideal scenario for the state pattern.
In the State pattern, your state classes should be responsible for transitioning state, not just checking the validity of the transition. In addition, pushing state transitions outside of the order class is not a good idea and goes against the pattern, but you can still work with an OrderProcessor class.
You should get each state class to implement the setQuantity operation. The state class should implement all methods that may be valid in some states but not in others, whether or not it involves a change of state.
There is no need for methods such as canChangeQuantity() and isValid() - the state classes ensure your order instances are always in a valid state, because any operation that is not valid for the current state will throw if you try it.
Your Order class's properties are stored with the order, not the state. In .Net, you'd make this work by nesting your state classes inside the Order class and providing a reference to the order when making calls - the state class will then have access to the order's private members. If you're not working in .Net, you need to find a similar mechanism for your language - for example, friend classes in C++.
A few comments on your states and transitions:
State A notes that the order is new, quantity is >0 and it has a product id. To me, this means you're either providing both of those values in the constructor (to ensure your instance starts off in a valid state, but you wouldn't need a setQuantity method), or you need an initial state which has an assignProduct(Int32 quantity, Int32 productId) method that will transition from the initial state to state A.
Similarly, you may want to consider an end state to transition to from state C, after the supplier has filled in the price.
If your state transition requires the assignment of two properties, you may want to consider using a single method that accepts both properties by parameter (and not setQuantity followed by set setProductId), to make the transition explicit.
I would also suggest more descriptive state names - for example, instead of StateD, call it CanceledOrder.
Here's an example of how I'd implement this pattern in C#, without adding any new states:
public class Order
{
private BaseState _currentState;
public Order(
Int32 quantity,
Int32 prodId)
{
Quantity = quantity;
ProductId = prodId;
_currentState = new StateA();
}
public Int32 Quantity
{
get; private set;
}
public Int32 ProductId
{
get; private set;
}
public String Supplier
{
get; private set;
}
public Decimal Price
{
get; private set;
}
public void CancelOrder()
{
_currentState.CancelOrder(this);
}
public void AssignSupplier(
String supplier)
{
_currentState.AssignSupplier(this, supplier);
}
public virtual void AssignPrice(
Decimal price)
{
_currentState.AssignPrice(this, price);
}
abstract class BaseState
{
public virtual void CancelOrder(
Order o)
{
throw new NotSupportedException(
"Invalid operation for order state");
}
public virtual void AssignSupplier(
Order o,
String supplier)
{
throw new NotSupportedException(
"Invalid operation for order state");
}
public virtual void AssignPrice(
Order o,
Decimal price)
{
throw new NotSupportedException(
"Invalid operation for order state");
}
}
class StateA : BaseState
{
public override void CancelOrder(
Order o)
{
o._currentState = new StateD();
}
public override void AssignSupplier(
Order o,
String supplier)
{
o.Supplier = supplier;
o._currentState = new StateB();
}
}
class StateB : BaseState
{
public virtual void AssignPrice(
Order o,
Decimal price)
{
o.Price = price;
o._currentState = new StateC();
}
}
class StateC : BaseState
{
}
class StateD : BaseState
{
}
}
You can work with your order processor classes, but they work with the public methods on the order class and let the order's state classes keep all responsibility for transitioning state. If you need to know what state you're in currently (to allow the order processor to determine what to do), you might add a String Status property on the order class and on BaseState, and have each concrete state class return its name.