Undo/Redo Implementation For Multiple Variables

Question

I'm trying to refactor an undo/redo implementation I have but am unsure how to go about it.

public class MyObject
{
    public int A;
    public int B;
    public int C;
}

public abstract class UndoRedoAction
{
    protected MyObject myobj;
    protected int oldValue;
    protected int newValue;

    public abstract void Undo();
    public abstract void Redo();
}

public class UndoRedoActionA : UndoRedoAction
{
    UndoRedoActionA(MyObject obj, int new)
    {
        myobj = obj;
        oldValue = myobj.A;
        newValue = new;
        myobj.A = newValue;
    }

    public override void Undo()
    {
        myobj.A = oldValue;
    }

    public override void Redo()
    {
        myobj.A = newValue;
    }    
}

public class UndoRedoActionB : UndoRedoAction
{
    UndoRedoActionB(MyObject obj, int new)
    {
        myobj = obj;
        oldValue = myobj.B;
        newValue = new;
        myobj.B = newValue;
    }

    public override void Undo()
    {
        myobj.B = oldValue;
    }

    public override void Redo()
    {
        myobj.B = newValue;
    }    
}

public class UndoRedoActionC : UndoRedoAction
{
    UndoRedoActionC(MyObject obj, int new)
    {
        myobj = obj;
        oldValue = myobj.C;
        newValue = new;
        myobj.C = newValue;
    }

    public override void Undo()
    {
        myobj.C = oldValue;
    }

    public override void Redo()
    {
        myobj.C = newValue;
    }    
}

Obviously, each of the UndoRedoAction child classes have custom functionality as they access different fields, but the functionality they perform on those fields are identical. Is there any clean way, short of making those ints into properties and passing property names (which I would rather not do, magic strings and such), to combine these into a generic UndoRedoAction instead of making a bunch of child classes that all perform the exact same actions on different variables?

I did consider using the Memento pattern which would solve the problem, but it seems quite overkill for a scenario this small and I have no one-way actions to worry about, which is where the Memento pattern is really useful.

Thanks.

Clarification: These UndoRedoAction objects are placed inside a Stack<UndoRedoAction> which serves as the undo cache. More specifically, there are two stacks, one for undo, one for redo, and actions popped off one are pushed onto the other. Additionally, in response to Zaid Masud's response, the variables are not necessarily all ints, or even all the same object type. My example merely did that for simplicity.

Answer 1

You can avoid reflection by using dynamic methods or expressions to create a custom property getter/setter based on a selector expression like x => x.SomeProperty.

Basically you'd want to do something like this:

public class PropertySetActionProvider<TObj, TProp>
{
    private Func<TObj, TProp> _getter;
    private Action<Tbj, TProp> _setter;

    public PropertySetActionProvider(Expression<Func<TObj, TProp>> propertySelector)
    {
        _getter = propertySelector.Compile();
        _setter = SetterExpressionFromGetterExpression(propertySelector).Compile(); 
    }

    public IUndoRedoAction CreateAction(TObj target, TProp newValue)
    {
        var oldValue = _getter(target);
        return new PropertySetAction<TObj, TProp>(_setter, target, oldValue, newValue);             
    }
}

public class PropertySetAction<TObj, TProp> : IUndoRedoAction 
{
   private Action<TObj, TProp> _setter;
   private TObj _target;
   private TProp _oldValue;
   private TProp _newValue;

   public PropertySetAction(Action<TObj, TProp> setter, TObj target, TProp oldValue, TProp newValue)
   {
        _setter = setter; 
        _target = target; 
        _oldValue = oldValue; 
        _newValue = newValue;
   }

   public void Do() 
   {
       _setter(_target, _newValue);
   }  

   public void Undo() 
   {
       _setter(_target, _oldValue);
   }   
}

Then you can easily create new actions with code like this:

  // create the action providers once per property you'd like to change
  var actionProviderA = new PropertySetActionProvider<MyObject, int>(x => x.A);
  var actionProviderB = new PropertySetActionProvider<MyObject, string>(x => x.B);

  var someObject = new MyObject { A = 42, B = "spam" };
  actions.Push(actionProviderA.CreateAction(someObject, 43);
  actions.Push(actionProviderB.CreateAction(someObject, "eggs");
  actions.Push(actionProviderA.CreateAction(someObject, 44);
  actions.Push(actionProviderB.CreateAction(someObject, "sausage");

  // you get the point

The only hard part is creating a setter expression from the getter expression (the SetterExpressionFromGetterExpression method above), but that's a known solved problem. See for example here and here for questions about how to do so.

In this approach the cost of compiling the getter/setter delegates from the expression is incurred only once when you create the action provider, not every time you create an action or invoke Redo or Undo on one.

If you want to further optimize, you can move the propertySelector parameter inside the action constructor, and have the action providers be created behind the scenes on demand and cached on a per-property basis. This will yield somewhat easier to use code but might be trickier to implement.

Hope this helps!