Context:

I'm trying to implement undo/redo using the Command pattern and WPF.

I have commands as properties of the ViewModel which manipulate the data. I bind to these commands in the View. (For context, I'm mostly manipulating a collection of objects with these commands. The collection is being used as the ItemsSource for a DataGrid)

Commands that can be undone implement this interface:

public interface IUndoableCommand : ICommand
{
    public void Undo();
}

I maintain a stack of these UndoableCommands. When an UndoableCommand is executed, it is pushed onto the stack. When I want to undo, I pop one of these commands off the stack, and executes its Undo() method.

Problem:

Implementing the Undo() method for many commands does not require any state to be remembered. I'm having trouble in the cases where they do.

For example, If I want to delete an item from the list with an UndoableCommand, I could store the deleted item inside the command object itself. Something like:

public class DeleteItemCommand : IUndoableCommand
{
    private object _deletedItem;

    public void Execute()
    {
        // Remove the item from the collection and set to _deletedItem
    }
    public void Undo()
    {
       // Insert the deletedItem back
    }
}

The problem with this is that I'm binding to the commands like this:

private IUndoableCommand _DeleteCommand;
public IUndoableCommand DeleteCommand
{
    get
    {
        if(_DeleteCommand == null)
        {
            _DeleteCommand = new DeleteItemCommand();
        }
        return _DeleteCommand;
    }
}

A new DeleteItemCommand instance is not created every time the command needs to be invoked. It's the same object. Storing the deleted item in this way wouldn't work.

I've looked around but I can only find examples of undo being implemented with the Command pattern outside the context of WPF DataBinding, or with Undo() methods that don't need state. I imagine it's a common scenario. Am I thinking about this all wrong? Is there some agreed upon solution to this problem?

The things I've thought about doing are either:

1. Creating a deep copy of the command and pushing that onto the undo stack instead.
2. Having the commands the View binds to create an instance of the command I actually want executed.

CodePudding user response：

The way I have handled Undo is to separate the UI-layer from the underlying undo mechanism. Just because "command pattern" and "ICommand" both have the work "command" does not mean they need, or should, be part of the same object hierarchy.

So your DeleteFromList-command could look like:

public class DeleteFromList<T> : IMyUndoObject{
    private T item;
    private List<T> list;
    public DeleteFromList<T>(T item, List<T> list) => (item, list) = (item, list);
    public void Undo() => list.Add(item);
}

and your wpf command like

public class DeleteLastFooCommand : ICommand{
    private List<Foo> foos;
    private MyUndoStack undoStack;
    public DeleteFooCommand(List<Foo> foos, MyUndoStack undoStack) => (foos, undoStack) = (foos, undoStack);
    public void Execute(){
        var removed = foos.Last();
        foos.RemoveAt(foos.Count - 1);
        undoStack.Push(new DeleteFromList<Foo>(removed, foos));
}

To actually undo an operation you would have a separate wpf command that kindly asks the undo stack to pop the last undo object and run it's undo method, if such an object is available.

This move the state to a separate object from your wpf command, so neatly avoids the problem.

However, I would also consider just saving the full state of your application. Each approach has its advantages, but it is likely that you want to be able to save the state regardless. Typically, this state would only be a few kilobytes, so you can probably maintain hundreds of undo states without needing to worry about memory.