This is now partially implemented (I have just committed it). There are many rough edges yet, but Configuration is now immutable, and it basically works. See:
https://github.com/apache/incubator-freemarker/blob/3/src/test/java/org/apache/freemarker/manualtest/GettingStartedExample.java Would be good if more people looks at this thing. As you will see if you do, there's a hierarchy of configuration related interfaces, which are implemented by both XxxConfiguration and XxxConfiguration.Builder. And so on... please find mistakes in this stuff, especially architectural ones. There are some missing and somewhat undecided things, most notably: - If a setting has a Map or List value, when someone calls builder.setFoo(someMap), should we copy someMap, or let people modify it via aliasing? Well, at the very latest when builder.build() is called, we must copy it, as the result must be immutable. Also, aliasing has this problem that when the user utilizes it, it's potentially confusing for someone else reading the code later, as instinctively you only consider the content of the Map (or List, etc.) where builder.setFoo(someMap) was called. Also if you utilize aliasing like builder.getFoo().put(some, more), then you possibly unwillingly assume that the Map passed into setFoo was mutable (not a Guava ImmutableMao.of(...), which is popular). So I'm bending towards copying the map at the setter, and return an immutable view with builder.getFoo(), but add a dedicated `builder.putFoo(some, more)` (as it was in FM2 as well). This allows us to do some performance tricks, like cache invalidation in Environment. - Should we allow calling builder.build() for multiple times? The trap one can run into with that is that then the two (or more) resulting objects will share the same stateful setting values, like the cacheStorage. That can lead to malfunction, and especially as cacheStorage has a default value, it's a quite sneaky thing (because, the user doesn't even realize that such setting exists, as he did not set it explicitly). Sunday, February 19, 2017, 7:41:54 PM, Woonsan Ko wrote: > On Sun, Feb 19, 2017 at 10:04 AM, Daniel Dekany <ddek...@freemail.hu> wrote: >> Sunday, February 19, 2017, 11:35:44 AM, Denis Bredelet wrote: >> >>> My message was not clear, I mean builders are not required for >>> standard configuration settings if they are simple types. >> >> Of course not. We are only talking about setting values like >> ObjectWrapper, TemplateLoader, TemplateResolver (when it's added), >> etc. For them, I think we will have just bite the bullet and add both >> `void setFoo(Foo)`, and `void setFooBuilder(Builder<Foo>)` (and their >> fluent API equivalent), and `Builder<Foo> getFooBuilder()`, but no Foo >> getFoo(). > > +1 > >> And if you call setFoo(myFoo), then getFooBuilder() will >> return something like a `new o.a.f.core.ResultWrapperBuilder(myFoo)`, >> where ResultWrapperBuilder is just a wrapper myFoo, so that it >> satisfies the Builder<Foo> interface. > > +1 > >> I think I will also ban using >> setFoo(Foo) if Foo is known to have a Builder, just to enforce the >> intended usage pattern. > > +1 > >> Unless somebody comes up with a better idea, I >> will try this approach when I get there, and we will see how it works >> out in reality. > > +1 > > Woonsan > >> >>>> Le 19 févr. 2017 à 10:24, Denis Bredelet <brede...@mac.com> a écrit : >>>> >>>> Hi Daniel, >>>> >>>>>>> But then we lose the immutability of the built object, which was the >>>>>>> point of using builders on the first place. >>>>>> >>>>>> Ah, you're totally right! If builder is able to update an immutable >>>>>> object, then the object turns out to be mutable indirectly through the >>>>>> builder. I missed that part. :-) >>>>>> Hmm.. it seems difficult to make it immutable considering those >>>>>> expensive nested objects. >>>>>> Would it be possible to introduce lifecycle methods such as >>>>>> #initialize() in Configuration to prohibit (IllegalStateException) >>>>>> using it and its nested objects until initialized? >>>>> >>>>> Sure, but that was plan B in case we don't go for builders at all (see >>>>> in the thread starter mail). I mean, if we have that, what's the >>>>> benefit of the builder classes? When I go for builders, I would hope >>>>> fore real final fields in the built objects, and that also means that >>>>> I don't have to worry about the ordering of the field assignments, >>>>> because I got all everything at once, so I can sort the assignments. >>>> >>>> Final fields for built objects is not a requirement if all the standard >>>> configuration settings are simple, e.g. String or Integer etc. >>>> >>>> Then possible mutations do not affect the standard configuration when >>>> using a builder. >>>> >>>> — Denis. >>>> >>>>>> If so, perhaps we can wait for everything configured in each stage >>>>>> and finally initialize those expensive objects only once by the >>>>>> core. Also, would it be possible to wait until the final >>>>>> initialization phase, only capturing settings, without having to >>>>>> create a nested object such as ObjecrWrapper in earlier stage? >>>>> >>>>> Where do I capture the settings of the ObjectWrapper (the nested >>>>> settings)? Anyway, that's why I thought that perhaps, if we have >>>>> cfg.setFoo(T) where Foo is not just some primitive or such, then we >>>>> also allow cfg.setFooBuilder(Builder<? extends T>)... and then, you >>>>> can, using your words, capture the nested settings too. >>>> >>> >>> >> >> -- >> Thanks, >> Daniel Dekany >> > -- Thanks, Daniel Dekany
