On Wed, Feb 15, 2017 at 2:54 PM, Woonsan Ko <woon...@apache.org> 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,
BTW, I'm not suggesting we should prefer fluent style API everywhere
(I'm kind of against changing everything to fluent style without good
reasons), but I'm just saying new Builders could be designed with that
style in mind.
Regards,
Woonsan
>
> 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
>>
