2

I would like to know if the code below is a good implementation of the state design pattern. Everywhere I see the state pattern to hold the context and a change in state doesn't return anything. But in my example below, I return the new state (i.e. North/east so on...). Please review and help.

public abstract class Direction
{
    public abstract Direction Right();


    public abstract Direction Left();
}


public sealed class East : Direction
{

    public East()

    { }


    public override Direction Left()
    {
        return new North();
    }


    public override Direction Right()
    {
        return new South();
    }

    public override string ToString()
    {
        return "EAST";
    }
}

Similarly for other directions.

Thanks a lot.

10
  • 1
    It already looks over-engineered to me. Have a look at this example: sourcemaking.com/design_patterns/state/java/1 – Robert Harvey Mar 11 '17 at 14:26
  • Ugg, that's also horribly hard-coded. – candied_orange Mar 11 '17 at 20:20
  • Here's an implementation of the state pattern. github.com/rubberduck203/VendingMachineKata – RubberDuck Mar 12 '17 at 0:29
  • @ CandiedOrange What's hard-coded here? Is there any other direction you would return apart from South if you are facing East and turn Right? By this logic I feel whole state design pattern itself it hard-coded. – Deepak Mar 12 '17 at 1:01
  • 2
    Well, you really don't need any of this. All you really need is a class with one state variable that holds the current state (it can be an enum), and a method that advances the state to the next valid state from the one currently stored in the state variable. You need one line of code for the state variable, and a switch statement with a case (two lines of code) for each state change. If the states can be represented by a monotonically increasing int, you don't even need the switch .The rest of the code in this pattern is just gratuitous boilerplate. – Robert Harvey Mar 12 '17 at 2:51
3

This is not an example of the state pattern, but you're almost there.

The problem with the way you have it now can be demonstrated with this example:

void Turn(Direction d, int degrees)
{
    switch (degrees)
    {
        case 90: d = d.Right();
        case -90: d = d.Left();
    }
}

void Example()
{
    Direction d = new East();
    Turn(d, 90);
    Turn(d, 90);
    Console.WriteLine(d.ToString());  //Prints East.  Should print West!!!
}

The issue is that the class doesn't actually contain state that tells you what direction it faces. It is the state. So if you change direction, somehow you have to go back and change every reference to the object to something else. if you don't do that, state is lost.

A correct state pattern implementation would include a container:

class DirectionState 
{
    private Direction _state;

    public DirectionState(Direction initialDirection) { _state = initialDirection; }

    public void Left()  { _state = _state.Left(); }
    public void Right() { _state = _state.Right();}
    public string ToString() { return _state.ToString(); }
}

Then this would work:

void Turn(DirectionState d, int degrees)
{
    switch (degrees)
    {
        case 90: d = d.Right();
        case -90: d = d.Left();
    }
}

void Example()
{
    DirectionState d = new DirectionState(new East());
    Turn(d, 90);
    Turn(d, 90);
    Console.WriteLine(d.ToString());  //Prints West
}
4
  • Thanks John for the reply. In my implementation use case of Direction class would be like Direction d = new East(); d= Turn(d, 90); //direction should return the new state. d= Turn(d, 90); Console.WriteLine(d.ToString()); // Now it gives west instead of east. So actor holds the responsibility of holding the current state. Thanks – Deepak Mar 16 '17 at 1:10
  • Sure, that would work. Just sayin, that is not the State Pattern. – John Wu Mar 16 '17 at 1:23
  • I think we both are talking the same thing here :) your DirectionState class is represented by another class(called ROBOT) in my program, so ROBOT saves the state and maintains it. – Deepak Mar 16 '17 at 12:32
  • Sorry I haven't included Robot class in the question. – Deepak Mar 16 '17 at 12:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.