On Fri, Feb 17, 2017 at 8:03 PM, Daniel Dekany <[email protected]> wrote:
> 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.
I see your point. Let me try for the plan A option with an example again. :-)
I guess it should look like this if we take all the existing requirements as-is:
//stage#1:
ConfigurationFactory configFactory = ConfigFactory.Builder.create(props)
.setObjectWrapperFactory(
DefaultObjectWrapperFactory.Builder.create()
.setIterableSupport(true)
//...
.build()
)
//...
.build();
//stage#n:
configFactory.getObjectWrapperFactory()
.setForceLegacyNonListCollections(true)
// ...
Configuration config = configFactory
.setDefaultEncoding("UTF-8")
// ...
.newInstance()
I've just tweaked the example with an additional Factories to build,
not the object directly. So, it's not creating the real Configuration
or ObjectWrapper, but the builder crates a new ConfigurationFactory or
ObjectWrapperFactory.
Could this work instead? Or do you think it's too complex?
>
>> 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.
That's fair. For Groovy, Kotlin or something similar, let's take
JavaBeans practices: `T setFoo(T)', `T getFoo()', `T isFoo()', ... (I
used this in the example above, too.)
Thanks for the pointer.
Regards,
Woonsan
>
>>>
>>>> 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
>
