Undo/redo functionality with LinkedList<Action> implementation

1.6k views Asked by At

I am writing my own 'Rubik's cube' application. The main class Cube has 18 rotation methods:

  • RotateAxisXClockWise, RotateAxisXAntiClockWise
  • RotateAxisYClockWise, RotateAxisYAntiClockWise
  • RotateAxisZClockWise, RotateAxisZAntiClockWise

  • RotateUpperFaceClockWise, RotateUpperFaceAntiClockWise

  • RotateFrontFaceClockWise, RotateFrontFaceAntiClockWise
  • RotateRightFaceClockWise, RotateRightFaceAntiClockWise
  • RotateBackFaceClockWise, RotateBackFAceAntiClockWise
  • RotateLeftFaceClockWise, RotateLeftFaceAntiClockWise
  • RotateDownFaceClockWise, RotateDownFaceAntiClockWise

Yes, they could be joined in pairs with a parameter Direction (for example RotateFrontFace(Direction direction)) but for now this seems appropriately.

I would like to implement undo/redo functionality and because all methods have the same signature (no input parameters, void return type) they could be saved in a LinkedList data structure. So every time one of the rotation methods is called, it is added to the linked list.

This would work pretty well if we start on the beginning of the LinkedList (haven't tried it out yet though) and advance toward the end, so each rotation would be performed exactly as it was in the first place.

But what about undo? If I traverse the list from the end to the beginnig, then the opposite method should be called (for example instead of RotateFrontFaceClockWise, RotateFrontFaceAntiClockWise should be called). Any ideas how to implement this? Elegantly? :)

4

There are 4 answers

1
LBushkin On BEST ANSWER

I would not use delegate references as the way to model the rotations, if one of the main purposes is to be able to perform redo/undo. I would consider creating a data-model for each rotation, and store a list of these rotation steps. Each step could then have it's own associated Redo/Undo delegate, which allows someone traversing the list (from either end) to understand what operations took place, and either repeat or reverse them.

One additional benefit of a data-oriented approach to model such transformations, is that it could potentially reduce the number of similar (but slightly different) versions of your RotateXXX( ) methods.

EDIT: Addressing the your question about what shape such a solution might take.

The simplest thing to do may be to store a Tuple<Action,Action> representing each pair of rotate/unrotate operations as paired delegates. However, I would consider using an explicit data structure that describes the rotation operation, perhaps eventually including things like a descriptive name, direction/face attributes, and so on. I would also change your RotateXXX methods so that they are static methods of Cube, and accept an instance of cube as a parameter. This would allow modeling the rotation operations externally to the instance of Cube.

public sealed class Rotation
{
    private readonly Action<Cube> _RotateAction;
    private readonly Action<Cube> _UnrotateAction;  // used for undo or backtracking

    private Rotation( Action<Cube> rotateAction, Action<Cube> unrotateAction )
    {
        _RotateAction = rotateAction;
        _UnrotateAction = unrotateAction;
    }

    public void Rotate( Cube cube )   { _RotateAction( cube ); }

    public void Unrotate( Cube cube ) { _Unrotate( cube ); }

    public static readonly RotateFrontFaceClockswise = 
        new Rotation( Cube.RotateFrontFaceClockwise
                      Cube.RotateFrontFaceCounterClockwise );

    public static readonly RotateFrontFaceCounterClockwise = 
        new Rotation( Cube.RotateFrontFaceCounterClockwise,
                      Cube.RotateFrontFaceClockwise );

    public static readonly RotateLeftFaceClockwise = 
        new Rotation( Cube.RotateLeftFaceClockwise,
                      Cube.RotateLeftFaceCounterClockwise );

    public static readonly RotateLeftFaceCounterClockwise = 
        new Rotation( Cube.RotateLeftFaceCounterClockwise,
                      Cube.RotateLeftFaceClockwise );
    // etc..
}

// now we can keep track of the state changes of a cube using:
List<Rotation> cubeRotations = new List<Rotation>();
cubeRotations.Add( Rotation.RotateFrontFaceCounterClockwise );
cubeRotations.Add( Rotation.RotateBackFaceClockwise );
cubeRotations.Add( Rotation.RotateLeftFaceCounterClockwise );

// to apply the rotations to a cube, you simple walk through the data structure
// calling the Rotate( ) method on each:
Cube someCube = new Cube( ... )
foreach( Rotation r in cubeRotations )
{
    r.Rotate( someCube );
}

// to undo these rotations you can walk the like in reverse:
foreach( Rotation r in cubeRotations.Reverse() )
{
    r.Unrotate( someCube );
}
0
LorenVS On

I know you wanted to avoid parameterizing your methods, but you're probably best off pursuing something along those lines:

enum Face
{
    Top,
    Bottom,
    Left,
    Right,
    Front,
    Back
}

enum Direction
{
    Clockwise,
    CounterClockwise
}

struct Rotation
{
     public Face Face;
     public Direction Direction;
}

LinkedList<Rotation> actions;

You can make a choice whether to push the direct or inverse action onto the list, but once you store the actions in this way, its very easy to write some quick switch statements to reverse an action as it gets removed from the list.

On a side note, consider replacing LinkedList with Stack, it will server you just as well and its suited for the purpose perfectly.

EDIT:

Noticed that I don't have support for the axis rotations in my enumerations, but I'm sure you could add them as additional entries in the Face enumeration (although you might want to rename it at that point)

2
PedroC88 On

Assuming you really want to stick to your model...

You could try by entering in the list every name of the method executed and then, with reflection, call the counter parts by iterating over the list. This assumes that your methods will have matching names (as you proposed).

You could also create a HybridDictionary into which you use the method names as the id and addressof the counter method as the value. And so when you iterate over the list you could have a delegate handle the method at the given value.

0
ryanpal On

18 methods seems like a lot to keep up with, especially when you consider implementing the undo/redo functionality. Have you considered using a single, more generic method? If you did so, you could store what was passed to that method in a very uniform manner, and easily perform the opposite actions. The opposite actions could be looked up from a dictionary.