[FR] Allow daisy-chaining when adding children to ValueTrees

Currently, it’s possible to daisy-chain property setters on a value tree:

juce::ValueTree {"Tree"}
    .setProperty("foo", 10, nullptr)
    .setProperty("bar", 20, nullptr);

It’d be great to have the behaviour for the child-adders as well, allowing a whole tree to be constructed this way:

juce::ValueTree {"Root"}
    .setProperty("foo", 10, nullptr)
    .setProperty("bar", 20, nullptr)
    .appendChild (juce::ValueTree {"Branch"}
                      .setProperty ("x", y, nullptr)
                      .appendChild (juce::ValueTree {"Leaf"}, nullptr),
                  nullptr)
    .addChild (juce::ValueTree {"Branch"}, 0, nullptr);

This is useful when loading value-trees from places external to the current call site (.e.g loading state from XML) and then adding on any additional children (e.g. any state that isn’t be persisted to the file).

It also allows for better const-ness as the tree doesn’t need to be changed after initialisation:

juce::ValueTree nonConst {"Tree"};
nonConst.appendChild (juce::ValueTree {"Data"}, nullptr);

const auto isConst = juce::ValueTree {"Tree"}
                         .appendChild (juce::ValueTree {"Data"}, nullptr);

PR here as the changes are very simple: Allow daisy-chaining when adding children to ValueTrees by ImJimmi · Pull Request #1332 · juce-framework/JUCE · GitHub

This would go nicely with [FR] Default UndoManager arguments to nullptr in all ValueTree methods - #6 by ImJimmi to make constructing value trees this way even cleaner:

juce::ValueTree {"Root"}
    .setProperty("foo", 10)
    .setProperty("bar", 20)
    .appendChild (juce::ValueTree {"Branch"}
                      .setProperty ("x", y)
                      .appendChild (juce::ValueTree {"Leaf"}))
    .addChild (juce::ValueTree {"Branch"}, 0);

One comment I have is that’s it’s a bit ambiguous as to whether the parent tree is returned or the newly added child.

Is there any precedent for this elsewhere in the juce library? Like XmlElement or similar?

Hmm true, I haven’t considered that!

Looks like the only method on juce::XmlElement that returns when adding a child is juce::XmlElement::createNewChildElement() which does in fact return a pointer to the newly added child.

Because of the ambiguity that Dave pointed out, it might be better to add a new member function to the existing class

ValueTree appendAndReturnChild(juce::ValueTree, UndoManager* = nullptr).

This would be simple and unambiguous (and it’s the perfect place to start using default null for UndoManager!)

I’d love to see some features like this added to ValueTrees. For such a simple class, it takes a surprisingly large amount of code to work with them, and there are many quick fixes that would really speed things up without breaking backwards compatibility.

1 Like

TBH, after reading it again I don’t think it is that ambiguous. Not always returning this would make it very hard to read and understand, since setProperty would return this.

Yeah good point. Returning this is pretty idiomatic.

i would never write such long lines anyway. just makes it terrible to debug

1 Like

It turns out that returning the child ValueTree isn’t as safe as it first seems.

If you did something like this…

    auto child = juce::ValueTree("parent")
        .addAndReturnChild(juce::ValueTree("child"), nullptr);

…the “parent” ValueTree would go out of scope, and you’d be left with an orphaned child. The existing API forces us to keep “parent” in scope, thus avoiding this problem.

See my other post here:

Daisy chaining is great, but also the ability to pass the undo manager once (which will lead to a terse syntax):

auto x = juce::ValueTree()
    .withChanges(undoManager)
        .setProperty("a", 1)
        .addChild(other, -1)
        ...