eo-yaml icon indicating copy to clipboard operation
eo-yaml copied to clipboard

Immutable builders cannot be used with optional and stream

Open filip26 opened this issue 4 years ago • 3 comments

Hi, I wonder why are builders immutable?

e.g. I would like to do:

final YamlSequenceBuilder sequence = Yaml.createYamlSequenceBuilder();
        
list.stream().map(....).filter(...).forEach(sequence::add);        

or:

final YamlMappingBuilder mapping = Yaml.createYamlMappingBuilder();
        
data.id().ifPresent(id -> mapping.add(Constants.ID, id));

but it's not possible because add method returns a new instance of a builder. FYI: JSON-P supports that. Only data structures and primitives are immutable.

Thank you

filip26 avatar Aug 21 '20 15:08 filip26

@filip26 well, all classes are immutable (or at least that's what we try to do), just because the design becomes cleaner and we also have thread-safety by default.

But YamlSequenceBuilder is an interface, maybe you can add your own mutable decorator, in some way?

amihaiemil avatar Aug 21 '20 15:08 amihaiemil

thank you for the quick response. Unfortunately, classes used internally by builders have package visibility and are final, so it's not possible.

I don't see any advantage having builders immutable. Even you get better performance if you make them mutable because you don't need to create new instances each time something is added to a sequence builder or to a map builder.

Isn't that the original purpose of builder pattern to allow you easily create and modify complex immutable objects?

e.g. snippet from RtYamlSequenceBuilder

    @Override
    public YamlSequenceBuilder add(final YamlNode node) {
        final List<YamlNode> list = new LinkedList<>();
        list.addAll(this.nodes);
        list.add(node);
        return new RtYamlSequenceBuilder(list);
    }

The code doesn't seem to be cleaner with immutable builders as well.

e.g. you have to do this to get the first example to work:

YamlSequenceBuilder sequence = Yaml.createYamlSequenceBuilder();

for (... item : list) {
    // map
   mappedItem = ...
   // filter
   if (...) {
      sequence = sequence.add(mappedItem)
   }
}

Please take this as a constructive input trying to help you to create a great Yaml library.

filip26 avatar Aug 21 '20 15:08 filip26

@filip26 I'm still pretty sure you can implement your own mutable builder based on the one returned by class Yaml.

I'll have a look this weekend, I can add it as an extension class if it works. But cureent builders stay as they are, sorry.

amihaiemil avatar Aug 21 '20 16:08 amihaiemil

@filip26 we implemented mutable builders for sequence and mappings in #512 and it will be released today with version 6.1.0

amihaiemil avatar Aug 27 '22 09:08 amihaiemil