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?
CodePudding user response:
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 ofQList<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));
}
}