0

This is my first C# application, entirely self-taught without any prior software programming background. I did some research on Undo/Redo but could not find anything helpful (or easy to understand). Therefore, I'm hoping someone can help me in designing undo/redo function for my program (winforms application). The application consist of a main form where subsequent child forms will be called to record user specified values during certain events (button clicks etc). After every event is handled, a bitmap will be drawn in buffer and then loaded to a picturebox within the main form during the OnPaint event of the main form. Each input in separated into custom class objects and added into separate List and BindingList. Objects contained within List are used for graphics (to indicate coordinates etc) while objects in BindingList are used to display some important values on DataGridView. Just to give a brief description, the codes look something like this:

public partial class MainForm : form
{
    public class DataClass_1
    {
        public double a { get; set; }
        public double b { get; set; }
        public SubDataClass_1 { get; set; }
    }

    public class SubDataClass_1
    {
        public double x { get; set; }
        public double y { get; set; }
        public string SomeString { get; set; }
        public CustomEnum Enum_SubDataClass_1 { get; set; }
    }

    public class DisplayDataClass
    {
        public string SomeString { get; set; }
        public double e { get; set; }
        public double f { get; set; }
    }

    public enum CustomEnum { Enum1, Enum2, Enum3 };

    // Lists that contain objects which hold the necessary values to be drawn and displayed
    public List<DataClass_1> List_1 = new List<DataClass_1>();
    public List<DataClass_2> List_2 = new List<DataClass_2>(); // Object has similar data types as DataClass_1
    public BindingList<DisplayDataClass> DisplayList = new BindingList<DisplayDataClass>();

    Bitmap buffer;

    public MainForm()
    {
        InitializeComponent();

        dgv.DataSource = DisplayList;
    }

    private void DrawObject_1()
    {
        // some drawing codes here
    }

    private void DrawObject_2()
    {
        // some drawing codes here
    }

    protected override void OnPaint(PaintEventArgs e)
    {
        DrawObject_1();
        DrawObject_2();
        pictureBox1.Image = buffer;
    }

    // Event to get input
    private void action_button_click(object sender, EventArgs e)
    {
        ChildForm form = new ChildForm(this);
        form.ShowDialog();
        Invalidate();
    }
}

The child forms' codes look something like this:

public partial class ChildForm : form
{
    public ChildForm(MainForm MainForm)
    {
        InitializeComponent();

        // Do something
    }

    private void ok_button_click(object sender, EventArgs e)
    {
        DataClass_1 Data_1 = new DataClass_1();
        DisplayDataClass DisplayData = new DisplayDataClass();

        // Parsing, calculations, set values to Data_1 and DisplayData

        MainForm.List_1.Add(Data_1);
        MainForm.DisplayList.Add(DisplayData);

        this.DialogResult = System.Windows.Forms.DialogResult.OK;
        this.Close();
    }
}

Since all necessary data are stored in the lists and will only be changed after certain events are triggered (mostly button clicks), therefore I tried to use these lists to determine the state of the application during run time. My approach in implementing the undo/redo function is by adding the following codes:

public partial class MainForm : form
{
    public class State()
    {
        public List<DataClass_1> List_1 { get; set; }
        public List<DataClass_2> List_2 { get; set; }
        public BindingList<DisplayDataClass> DisplayList { get; set; }
        // and so on

        public State()
        {
            List_1 = new List<DataClass_1>();
            List_2 = new List<DataClass_2>();
            DisplayList = new BindingList<DisplayDataClass>();
        }
    }

    State currentState = new State();
    Stack<State> undoStack = new Stack<State>();
    Stack<State> redoStack = new Stack<State>();

    private void MainForm_Shown(object sender, EventArgs e)
    {
        // Saves original state as first item in undoStack
        undoStack.Push(currentState);            
    }

    protected override void OnPaint(PaintEventArgs e)
    {
        // Update lists from currentState before drawing
        List_1 = new List<DataClass_1>(currentState.List_1);
        List_2 = new List<DataClass_2>(currentState.List_2);
        DisplayList = new BindingList<DisplayDataClass>(currentState.DisplayList);
    }

    // When undo button is clicked
    private void undo_button_Click(object sender, EventArgs e)
    {
        if (undoStack.Count > 0)
        {
            redoStack.Push(currentState);
            undoStack.Pop();
            currentState = undoStack.Last();
            Invalidate();
        }
    }

    // When redo button is clicked
    private void redo_button_Click(object sender, EventArgs e)
    {
        // Have not thought about this yet, trying to get undo done first
    }

    // Events that trigger changes to values held by data objects
    private void action_button_Click(object sender, EventArgs e)
    {
        // Replace the following code with previously stated version         
        if (form.ShowDialog() == System.Windows.Forms.DialogResult.OK)
        {
            ChildForm form = new ChildForm(this)
            UpdateState();
            undoStack.Push(currentState);
            Invalidate();
        }
    }

    // To update currentState to current values
    private void UpdateState()
    {
        currentState.List_1 = new List<DataClass_1>(List_1);
        currentState.List_2 = new List<DataClass_2>(List_2);
        currentState.DisplayList = new BindingList<DisplayDataClass>(DisplayList);
        // and so on
    }
}

Result: The application does not perform the undo function correctly. The program shows the correct output under normal conditions but when the undo event is triggered, regardless of how many objects have been drawn, the application reverts back to initial state (the state where there is no recorded data). I've used System.Diagnostics.Debug.WriteLine() during events where the stack is changed to check the number of counts within undoStack and it seems to give the correct counts. I'm guessing that the lists need to be copied/cloned in a different manner? Or am I doing something wrong here? Can anyone please guide me? Performance, readability, resource management, future maintenance and etc need not be considered.

  • 4
    This is way too much code for a stack overflow post – debracey Apr 26 '12 at 23:29
  • In addition to this being way too much code, it reads like one big blob of text with a bunch of code stuffed in. You can use paragraph breaks here so your text isn't one big block - it makes it much easier to read and understand. (You can preview your post immediately below where you're writing it in what's pretty much WYSIWYG fashion, so you can see how it appears before submitting the question.) I tried to wade through the volumes of text, but they were simply too hard to read. Please edit your question, properly write paragraphs, and whittle it down to the *least* possible content.) Thanks. – Ken White Apr 26 '12 at 23:33
  • 3
    For what it's worth, undo/redo is usually implemented using the [Command pattern](http://en.wikipedia.org/wiki/Command_pattern). Rather than storing the entire state in the undo stack, you store the action that led to this state. – Thomas Levesque Apr 26 '12 at 23:57
  • 1
    You may want to look at examples like: [Undo/Redo Framework](http://www.codeproject.com/Articles/19550/Automated-Undo-Redo-library-Csharp-Generics-NET-C) or [Undo/Redo Buffer](http://www.codeproject.com/Articles/10576/An-Undo-Redo-Buffer-Framework). You may also want to read Martin Fowler's article on [temporal tracking](http://martinfowler.com/eaaDev/timeNarrative.html). – LBushkin Apr 27 '12 at 01:15
  • 1
    You may want to look at http://www.codeproject.com/Articles/456591/Simple-Undo-redo-library-for-Csharp-NET. It seems to fit your needs. Regards. – user1668703 Sep 17 '12 at 23:03

2 Answers2

3

There are a lot of approaches that will work, each with different strengths and weaknesses, but I generally like to define an abstract Action class and then a separate UndoRedoStack class.

The Action class would have two methods (Do and Undo) which each subclassed Action can implement. You isolate any logic that can "change state" to these Action subclasses thereby keeping that logic neatly encapsulated.

The UndoRedoStack is like a regular stack except with three core methods.

  1. ApplyAction (like Push)
  2. UndoAction (like Pop, but be sure to only move the pointer/index without truncating or throwing away any existing actions).
  3. RedoAction (like Push, but you use the next value already in the underlying stack/list instead of pushping/inserting a new one).

Usually I find the biggest challenge then becomes designing each Action subclass in such a way that it maintains enough information to both undo and redo itself. But being able to encapsulate all state manipulation logic to individual Action subclasses usually makes it easiest for me to maintain in the long run.

C. Dragon 76
  • 9,882
  • 9
  • 34
  • 41
  • 1
    Using delegates is a good alternative to implementing subclasses of Action for each different action (see [this article](http://tommulgrew.pixelati.com/2011/04/23/implementing-doundo-support-in-net-applications-with-lambda-expressions/) for details) – Thomas Levesque Apr 27 '12 at 00:01
0

You are storing reference objects in your stacks. If you want your method to work, you need to implement a clone() method in your state object, and store a new clone each time, otherwise, changes made are made to each member of the stack, as they all point to the same reference object.

Wanabrutbeer
  • 676
  • 6
  • 11