Friday, February 17, 2017, 11:48:34 PM, Woonsan Ko wrote: > On Fri, Feb 17, 2017 at 4:09 PM, Daniel Dekany <[email protected]> wrote: >> Friday, February 17, 2017, 3:16:28 PM, Woonsan Ko wrote: >> >>> On Fri, Feb 17, 2017 at 4:22 AM, Daniel Dekany <[email protected]> wrote: >>>> Friday, February 17, 2017, 6:52:15 AM, Woonsan Ko wrote: >>>> >>>>> On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany <[email protected]> >>>>> wrote: >>>>>> One other problem I see with builders is that they seem to be >>>>>> difficult to work with when it comes to working out the Configuration >>>>>> setting values in multiple stages. >>>>>> >>>>>> What do I mean by multiple stages? For example, the user specifies >>>>>> some settings in the web.xml in the case of FreemarkeServlet, so >>>>>> applying those is stage one. Then the Configuration builder is worked >>>>>> on by FreemarkeServlet itself, which adjusts it further, usually by >>>>>> setting some defaults where the setting wasn't set in the previous >>>>>> stage. Then it goes down one more stage, to FreeMarker itself, where >>>>>> the FreeMarker defined defaults are applied to the settings which were >>>>>> not set so far. You may say that it should be done on the opposite >>>>>> order, that is, FreeMarker fills its defaults, then FreemarkerServlet >>>>>> overwrites some setting with its own defaults, then comes the web.xml >>>>>> content. That's simpler to do, but has some problems. One is that you >>>>>> may construct setting values that will be later overwritten, and not >>>>>> all setting values are cheap to construct (unless... for complex value >>>>>> we require passing in a builder instead of the object). The other is >>>>> >>>>> Another option I'm thinking is to extend the Builder pattern to be >>>>> able to use an existing object instead of new creation when needed. >>>>> As an example pseudo code, >>>>> >>>>> // stage #1: >>>>> Configuration config = Config,Builder.create(props) // create new >>>>> .templateUpdateDelayMilliseconds(60000) >>>>> .defaultEncoding("UTF-8") >>>>> //... >>>>> .build(); >>>>> >>>>> // stage #2: >>>>> config = Config.Builder.update(config) // update existing config >>>>> .whitespaceStripping(true) >>>>> //... >>>>> .build(); // return the same config instance after updates >>>>> >>>>> // stage #n: >>>>> // ... >>>>> >>>>>> that you may want to adjust settings nested inside values (such as the >>>>>> settings of an ObjectWrapper), so the top-level setting must be >>>>>> already set by the higher layer when it gets to you. Anyway, whichever >>>>>> order you chose, this is the use case. >>>>> >>>>> Nested one can also create or update (or createOrUpdate sometimes?), IMHO: >>>>> >>>>> // stage #1: >>>>> Configuration config = Config,Builder.create(props) >>>>> .objectWrapperBuilder( >>>>> // new builder to create a new objectWrapper >>>>> DefaultObjectWrapper.Builder.create() >>>>> .iterableSupport(true) >>>>> //... >>>>> .build() >>>>> ) >>>>> //... >>>>> .build(); >>>>> >>>>> // stage #2: >>>>> config = Config.Builder.update(config) >>>>> .objectWrapperBuilder( >>>>> // new builder to update existing objectWrapper >>>>> >>>>> DefaultObjectWrapper.Builder.update(config.getObjectWrapper()) >>>>> .forceLegacyNonListCollections(true) >>>>> //... >>>>> .build() >>>>> ) >>>>> //... >>>>> .build(); >>>> >>>> That (recreate Builder from the built object) is a possibility. But >>>> note that it comes with some extra price over the obvious extra >>>> complexity it adds: >>>> >>>> - Objects are fully built, then dropped, then rebuilt... It almost never >>>> matters in out case, but creating some setting values, like creating >>>> some custom TemplateLoader-s or ObjectWrapper-s, can be relatively >>>> expensive (like it involves I/O or permanent allocation of bigger >>>> static data structures). So it's not an universally applicable >>>> pattern at least. >>> >>> Oh, that's not what I meant. >>> In the first example, >>> >>> // stage #2: >>> config = Config.Builder.update(config) // update existing config >>> .whitespaceStripping(true) >>> //... >>> .build(); // return the same config instance after updates >>> >>> the first line takes an existing Configuration object only to update >>> and return it again only after setting more properties. >>> So, the return is meaningless here, and #build() means 'updating >>> build', not 'creation build', which I think can be regarded as an >>> extended Builder pattern. >> [snip] >> >> 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. > 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. >>> Yeah, I'm fine with `T getFoo()' or `boolean isFoo()'. But if it >>> follows a fluent style, it's already free from the JavaBeans >>> conventions anyway. >> >> Many frameworks/tools look for JavaBean properties. Like Spring IoC >> for example, if you are using the XML configuration. They couldn't >> get/set the properties through a fluent API. > > I have assumed freemarker-spring layer on top of the core API > including builders. freemarker-spring could follow JavaBean > properties. JavaBeans properties are used by many things, not just Spring. Like some of modern languages (like Groovy and Kotlin as far as I remember) allow you to write `foo.x = 5` which translates to `foo.setX(5)` in Java. >> >>> The exception in fluent Builders seems fine to me. >> >> (What exception do you mean?) > > Just allowing fluent style for Builders in the core api, not JavaBeans > style. That's what I meant by 'exception'. > > Woonsan > >> >>> Regards, >>> >>> Woonsan >>> >>>> >>>>> Kind regards, >>>>> >>>>> Woonsan >>>>> >>>>>> >>>>>> Any thoughts? Especially on what to do with those nested settings. >>>>>> >>>>>> >>>>>> Wednesday, February 15, 2017, 8:54:52 PM, Woonsan Ko wrote: >>>>>> >>>>>>> On Wed, Feb 15, 2017 at 5:48 AM, Daniel Dekany <[email protected]> >>>>>>> wrote: >>>>>>>> Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote: >>>>>>>> >>>>>>>>> Also, if possible, I would like to have an immutable object or >>>>>>>>> representation than the Configuration itself. For example, >>>>>>>>> template#getConfiguration doesn't have to return a mutable object, for >>>>>>>>> instance. But it requires a bigger change, I guess. >>>>>>>> >>>>>>>> A good point, that's the totally correct way of doing things. So we >>>>>>>> could have a mutable Configuration.Builder, which yields an immutable >>>>>>>> Configuration. We also have some ObjectWrapper, which is a bean in >>>>>>>> itself, but is also part of the Configuration. We certainly will have >>>>>>>> another such "injected bean", the TemplateResolver, and maybe some >>>>>>>> more on long term. Those also could follow this pattern (i.e, being >>>>>>>> immutable and only offer a builder). >>>>>>>> >>>>>>>> Question is, what extra pain that means for the average user. Like >>>>>>>> then in Spring XML you have to create a factory bean from Xxx.Builder, >>>>>>>> and then create yet another bean with factory-bean and factory-method >>>>>>>> attribute (because for some reason javax.annotation doesn't have >>>>>>>> anything like Spring FactoryBean, and freemarker-core can't depend on >>>>>>>> Spring obviously). Or, we can subclass all builders in something like >>>>>>>> freemarker-spring, to create a FactoryBean version of them, and then >>>>>>>> tell people to (a) pull in the depednecy and (b) use those classes >>>>>>>> instead of those shown in the core JavaDoc and in the Manual >>>>>>>> examples... So it's not very streamlined. If you have traditional >>>>>>>> beans, it's simpler for the users (especially if they don't have a >>>>>>>> PostConstruct either). Note sure which compromise is the winner. >>>>>>> >>>>>>> From spring users' perspective, I don't think it would be a problem >>>>>>> for them to add one more dependency on freemarker-spring to use the >>>>>>> factory beans (dedicatedly designed for spring). And, they have >>>>>>> already been doing it with others (JndiObjectFactoryBean, >>>>>>> MBeanServerFactoryBean, FreemarkerConfigurationFactoryBean, etc.). >>>>>>> It might be a little bit more work from development perspective, but I >>>>>>> don't see anything weird from end users' perspective by having >>>>>>> freemarker-spring and FactobyBean layers. >>>>>>> Also, in general, I think it's a lot safer and more resilient in >>>>>>> upgrade paths to use a factory bean instead of allowing to access >>>>>>> mutable bean directly because we can safely control everything through >>>>>>> the factory. >>>>>>> >>>>>>> Anyway, I like the idea having Builders which could perhaps look like >>>>>>> a fluent Java DSL. For example, >>>>>>> >>>>>>> Configuration config = ConfigBuilder.create(props) // ConfigBuilder >>>>>>> or Config.Builder >>>>>>> .templateUpdateDelayMilliseconds(60000) >>>>>>> .defaultEncoding("UTF-8") >>>>>>> //... >>>>>>> .build(); >>>>>>> >>>>>>> I think this will help people using Java API directly. And, I think >>>>>>> it's also a good idea to have FactoryBeans (using the Builder APIs) in >>>>>>> freemarker-spring for their convenience. >>>>>>> >>>>>>>> >>>>>>>> If Configuration (and the others) remains a bean, I want to throw an >>>>>>>> IllegalStateException if a the object were already "used" (like >>>>>>>> template processing was already invoked), and then someone calls >>>>>>>> cfg.setXxx(value). In FM2 we say that if you do that from multiple >>>>>>>> threads, then it won't work correctly, but we allow changing the >>>>>>>> Configuration at any time if you are only using it from the same >>>>>>>> thread. That's a possibility I want to remove, even if we don't switch >>>>>>>> to builders. >>>>>>> >>>>>>> Runtime exception option sounds like less attractive to me than >>>>>>> factory beans. I would happily accept freemarker-spring library to >>>>>>> take advantage of all the good factory beans. >>>>>>> >>>>>>>> >>>>>>>> Oh, and yet another complication with builders... If Configuration is >>>>>>>> immutable, then it only will have getters, no setters. But, >>>>>>>> Environment is mutable with setters, and it should be. Both >>>>>>>> Configuration and Environment extends Configurable. So it seems now we >>>>>>>> should split that class into a getters-only class and a subclass that >>>>>>>> adds setters... (That might will be necessary because that Template is >>>>>>>> mutable with setters is not all right either. Or we can just throw >>>>>>>> IllegalStateException there too.) >>>>>>> >>>>>>> Yeah, I would separate those. >>>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> Woonsan >>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Thanks, >>>>>>>> Daniel Dekany >>>>>>>> >>>>>>> >>>>>> >>>>>> -- >>>>>> Thanks, >>>>>> Daniel Dekany >>>>>> >>>>> >>>> >>>> -- >>>> Thanks, >>>> Daniel Dekany >>>> >>> >> >> -- >> Thanks, >> Daniel Dekany >> > -- Thanks, Daniel Dekany
