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. > 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. > The exception in fluent Builders seems fine to me. (What exception do you mean?) > 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
