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
>
