Friday, May 12, 2017, 10:49:33 PM, Daniel Dekany wrote: > Thursday, May 11, 2017, 6:49:09 AM, Woonsan Ko wrote: > >> Hi Daniel, >> >> On Fri, May 5, 2017 at 3:18 PM, Daniel Dekany <ddek...@apache.org> wrote: >>> 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. >> >> It looks really good! Great work! >> I think a builder instance may return the same instance. I've read >> about the builder pattern again: >> https://en.wikipedia.org/wiki/Builder_pattern. There's no rule or >> reason to have to return a new instance on every #build() call. Even >> the Java example, CarBuilderImpl, in the page returns the same >> instance, too. It seems a lot safer to do in our context as well. So, >> I would return the same instance from the same builder on #build() >> calls. > > But that's impossible if the builder creates immutable objects (which > is the main reason of using builders in many Java frameworks, > including FM3): > > Object obj1 = builder.build(); > b1.setFoo(123); > Object obj2 = builder.build(); > assertSame(obj1, obj2) // Impossible, as we create immutable objects
Any reaction to this? Right now I have no better idea than disallowing calling build() for more than once (by throwing IllegalStateException). >> Kind regards, >> >> Woonsan >> >>> >>> >>> 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 >>> >> > -- Thanks, Daniel Dekany
