Best practises using ValueTree + UndoManager?

General JUCE discussion
1

Is the following bad practice when using ValueTrees with UndoManager?

  1. ValueTree represents an instance of a class.
  2. You give that ValueTree to the class constructor.
  3. The constructor adds its own properties and child nodes IF they are missing.

At first look that would sound like it would always work OK in all situations. But then there’s the following “grey area” case:

  1. You have a parent ValueTree.
  2. Children added to the parent ValueTree represent the classes you want to construct.
  3. You use ValueTreeListener to automatically call valueTreeChildAdded() whenever you add a new child ValueTree to the parent.
  4. valueTreeChildAdded() is responsible of creating the instance of the class, calling its constructor.
  5. So valueTreeChildAdded() calls the class constructor by giving it the new child ValueTree as a parameter.

Now if the class constructor is designed the way the very first list describes in this post, that leads to a situation where you tamper with ValueTree properties/children during valueTreeChildAdded() call. That in turn affects UndoManager operation history as it adds new things to undo/redo.

In practice that doesn’t seem to cause any trouble, as when you create that class instance the very first time, you’ll add all those extra properties/child nodes. Now if you delete that ValueTree child from its parent and then use UndoManager to redo all those steps, the constructor don’t need to mess with any of the ValueTree properties / children anymore as they’re already present. So the undo-history won’t be affected.

But in practice that feels like a source of potential bugs as the architecture can feel somewhat convoluted, unless you know all the above about how UndoManager works and how all those properties/etc. get initialized only on the first run of that constructor. But its really easy to forget to keep all that in mind while designing new classes.

Is there some best practice how to structure your architecture using ValueTrees with UndoManager, to make it as clear, simple, easy to read and fool proof as possible?

The most obvious way to do it would be to never ever do any changes to any of the properties nor ValueTrees inside valueTreeChildAdded(), valueTreeChildRemoved(), etc. That would mean that the constructors of classes weren’t allowed to add their own items into ValueTrees anymore. That in turn would mean that before valueTreeChildAdded() (etc.) gets called by the ValueTree listener, the class specific properties/children would have to have been added into the ValueTree already. I.e. the code outside of that class constructor would need to know some specifics of how the internals of that class work. Otherwise the outside code wouldn’t know which properties / children to add to the ValueTree, before adding it to its parent, which in turn then calls the constructor.

Any thoughts on this topic?

2

I think a good way of dealing with this is to use CachedValue, which can hold a default value. That way, the constructor doesn’t need to alter the ValueTree it just specifies the default value which can be accessed via CachedValue even if the ValueTree property isn’t there.

More broadly, what I usually do is to create a wrapper class around the ValueTree which is passed by shared_ptr. Similar to ValueTree itself, the wrapper class listens to the ValueTree and takes care of adding and removing children, and then implements its own custom listener callback system. This allows you to separate the ValueTree logic (which is often quite fussy) from the behaviour you want your own type to have. It’s not always necessary, but if your type is getting complicated, it can be a lifesaver.