On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany <ddek...@freemail.hu> 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();
>
> The problem is with the configuration setting values that are
> themselves beans and was (potentially) built with a Builder
> (ObjectWrapper-s is a good example). I pass the Configuration.Builder
> to the next stage, but there you can't get the builder of the
> ObjectWrapper anymore there, only the read-only ObjectWrapper. So you
> can't customize the settings of that further. With other words, you
> can't customize nested settings, only the top level ones, which
> strikes me as a quite arbitrary restriction. Unless, I also allow
> setting the value of a configuration setting to a builder object
> instead of the value object itself... which starts to be
> overcomplicated, because you can now set the same setting either by
> specifying a builder or the result object. Not sure if that's
> acceptable.
Can we perhaps consider to extend Builder pattern to be able to either
create or update like the examples above?
>
> BTW, with Builder-s you are lucky if you can get back the property
> values that you or someone else has set earlier at all. Many thinks
> that they should be write-only objects. I think it's overly
> restrictive. So as far as I'm concerned, the builder should these
> methods for each setting foo (for a simple value this time):
> - void setFoo(T value)
> - Builder foo(T value) // Some prefer withFoo... I don't.
> - T getFoo() // Design pattern junkies raise eyebrows here ;)
> - boolean isFooSet() // This is how you know that you can supply the
> // default. Called isFooExplicitlySet() in FM2.
I have preferred 'Builder foo(T value)' and 'T foo()' simply, but I'll
all ears. ;-)
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 <ddek...@freemail.hu> 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
>
