Qt C++ Memento Design Pattern, I am trying to add a Undo/Redo function to my program, but why doesn't it work properly?

Question

I am learning about the Memento Design Pattern and have constructed a simple Program for this purpose.

I have constructed 2 classes, Container, which only holds a QString called Code

The GUI is very simple, there is a QListWidget that displays a list of Container Items that have not been allocated to an Pallet Object, i.e. unallocated.

The Pallet class contains a QList of type Container, which would be allocated.

The MainWindow class has both a QList of type Container and Pallet, the former holding the previously mentioned unallocated Container Items.

The unallocated Items are displayed in a QListWidget Object, and a new Container item is appended whenever a specific QPushButton is pressed.

When adding an unallocated item to a Pallet, the number of which is determined by a QSpinBox, the item disappears from the list and is stored in that Pallet's QList<Container*>.
At the same time, the Pallet's information is displayed in a QTextEdit on a seperate Tab.

My Problem is that when I press the Button assigned to undo the last step, it successfully undoes the changes to the unallocated QList, but not to the Pallet QList.

Here are the Code Fragments that deal with my attempt at the Memento Design Pattern:

//memento class

class Memento
{
private:
    Memento();

    friend class MainWindow;

    void setPallet_State(QList<Pallet*> PList);
    void setUnallocated_State(QList<Container*> CList);

    QList<Pallet*>  getPallet_State();
    QList<Container*> getUnallocated_State();

    QList<Pallet*> Pallet_State;
    QList<Container*> Unallocated_State;
};

//memento implementation

Memento::Memento()
{
}

void Memento::setPallet_State(QList<Pallet *> PList)
{
    Pallet_State = PList;
}

void Memento::setUnallocated_State(QList<Container *> CList)
{
    Unallocated_State = CList;
}

QList<Pallet *> Memento::getPallet_State()
{
    return Pallet_State;
}

QList<Container *> Memento::getUnallocated_State()
{
    return Unallocated_State;
}

Here are some of the cases where I implemented the Memento class in the MainWindow:

//inside of MainWindow class
public:
...
    void move_to_Pallet();
    Memento createMemento();
...
    Memento MEMENTO = Memento(); //caretaker for when undoing
    Memento RESTORE_MEMENTO = Memento(); //caretaker for redo
    void RestoreMoment(Memento X);
    void runUndo();
    void runRedo();
...

Note: the '...' in the code above is arbitrary and had nothing to do the the problem.

//implementation of above functions

void MainWindow::move_to_Pallet()
{
    //creating a memento in case of Backup
    MEMENTO = createMemento();
    ...
    //this is supposed to create a memento in case I need to redo
    RESTORE_MEMENTO = createMemento();
}

Memento MainWindow::createMemento()
{
    QList<Pallet*> Pallet_List;
    Pallet_List = PAL_List;
    QList<Container*> U_Container;
    U_Container = Unallocated;

    Memento moment;
    moment.setPallet_State(Pallet_List);
    moment.setUnallocated_State(U_Container);

    return moment;
}

void MainWindow::RestoreMoment(Memento X)
{
    PAL_List.clear();
    PAL_List = X.getPallet_State();
    Unallocated.clear();
    Unallocated = X.getUnallocated_State();

    print_Lists(); //this just displays the information within the MainWindow's Pallet QList and  
                   //it's contents
}

void MainWindow::runUndo()
{
    RestoreMoment(MEMENTO);
}

void MainWindow::runRedo()
{
    RestoreMoment(RESTORE_MEMENTO);
}

The Problem that I have is that when I add only one Container Item from the 'unallocated' list to a pallet, i.e. the very first item of the pallet, the undo/redo function works as intended with any number of pallets, once more items are added to pallets, the MainWindow's Pallet QList does not seem to update as it should.
In fact, it can duplicate items sometimes.

Here is a quick example: Before Undoing, this is what the unallocated list and the Pallet List looks like:

And this is what this becomes after I run the Undo function

What am I doing wrong here?

score 1 · Accepted Answer · answered Aug 25 '22 at 13:45

Please notice: the problem lies in the fact you're using a list of pointers to store the pallets.

It goes like this:

pallet list has 2 items
before modifying it you copy the list to the memento
then you add a container to the last of the two items

At this point you should notice that the list of pallets isn't really changed: same two pointers there. What changed (and wasn't saved in the memento) is an extra item in the internal list of containers held by the last pallet in the pallets list. All that was saved in the memento, instead, was a list of two pointers, and that's what you'll have back when you use the memento to restore the previous state.

Now, just to make it work as you intended, you have two choices:

change the way to represent your state and use QList<Pallet> instead of QList<Pallet *>
change the way to save your state in the memento, thus making a deep copy of the pallets list each time:

void Memento::setPallet_State(QList<Pallet *> PList)
{
    Pallet_State.clear();
    for(auto p : PList)
    {
        Pallet_State.append(new Pallet(*p));
    }
}

Qt C++ Memento Design Pattern, I am trying to add a Undo/Redo function to my program, but why doesn't it work properly?

1 Answers1