Should I build undo stack in model or its wrapper

api-designdesign-patternspatterns-and-practicesqt

I am building an application (Python/PyQt). The first-order item will be a tree view/model with many helper functions to add new items, move them around in the tree, etc.. Then I will have a main window object that will basically serve as a wrapper for the tree. This main window has a toolbar with buttons for the user to add/remove/move items (these buttons call the helper methods built into the tree object).

It's just about done, but alas I have decided to add undo/redo functionality using Qt's undo framework. It is my first time using undo/redo functionality, and I am curious whether there is consensus/experience on whether to implement the undo stack in the main window wrapper, or push the details into the internals of the tree class. I can see going either way.

Initially I put the undo stack functionality into the wrapper, so I could keep my tree simpler, and I was thinking that undo/redo is largely a sort of simple wrapper itself, not something I wanted to include in the functionality of the tree itself.

As I started, though, I realized I seemed to be breaking modularity, and because of the way these undo classes are implemented (they are fairly complicated and get into the internals of the tree model), it was starting to feel like I was making the wrapper concern itself with too many details of how the tree is implemented. This defeats the purpose of a simple wrapper.

So now I'm leaning toward incorporating the undo stack, in particular QUndoCommand classes, into the tree model, which is happy to deal with all these internals. This would likely simplify the wrapper, which is sort of the point. In other words, I'm thinking that I was naive about how complicated it was to add undo/redo functionality. It isn't simple, but something the model itself really should be working on.

However, I am completely new to this, so am wondering if I am making a silly design error that will haunt me in two years when I try to expand the project. One thing I appreciate now: the way I incorporate undo/redo into this application will be a pain to switch in the future, so I want to do it right the first time.

Note added

Based on a comment, I searched 'undo design patterns' and got some potentially relevant links:

If you look at the docs for Qt's undo framework linked above, it seems to use the command design pattern, so that's what I'm working with. So reposing my question in that light, should I generate the command objects within the original class, or its wrapper? Is there any obvious flaw in my above reasoning that it is better for modularity to do it in the original class?

Update added in 2020
I implemented the undo/redo functionality in the main class, not its wrapper and this was clearly the right move.

Best Answer

It seems that you are not asking how to implement undo, since you've already chosen to work with Qt's Undo Framework, which already has an implementation to hook in to.

If I understand you correctly, you are more interested in where to implement the necessary code. It seems that you have tried implementing it in two places which leads to complexity in either situation (been there before). So in some ways it is an arbitrary choice.

One thing you may not have considered is creating more than one wrapper. If you were to wrap your tree with a wrapper which added the undo functionality and then another wrapper which adds additional functionality your application requires, then you may end up with a better architecture. Kind of in line with the Single Responsibility Principle. Note you may wrap the tree in a different order, depending on your needs.