freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Dekany (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (FREEMARKER-39) DefaultObjectWrapperBuilder isn't a Builder
Date Sun, 06 Nov 2016 09:51:58 GMT

    [ https://issues.apache.org/jira/browse/FREEMARKER-39?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15641502#comment-15641502
] 

Daniel Dekany edited comment on FREEMARKER-39 at 11/6/16 9:51 AM:
------------------------------------------------------------------

It is a builder all right, it just doesn't provide a fluent API. I know it's common for builders
nowadays, but having a fluent API is not the real point of using builders (like you had builders
before Java 5 as well), that's an extra for aesthetics. Anyway, then your point is that should
provide a fluent API. Therefore I will change this from a bug to RFE, and I hope you don't
mind.

The setters can't have a return value, because that conflicts with the JavaBeans specification.
But fluent API-s usually use {{foo(value)}} instead of {{setFoo(value)}} anyway, so luckily
both can exist on the same time. So no problem so far.

To add those {{foo(value)}} methods, we need the usual self-type generics hack, as you mention.
When these things were added, Java 5 wasn't a requirement, so the fluent API thing was out
of question. Now I guess they could be added, though frankly, I don't think users spare much
time with it... how often does one configure FreeMarker? And then, the same need will arise
with the {{Configuration}} API as well (and for all the others). We also have to consider
backward compatibility constraints, though at the first glance we only generate raw type warnings
here. But that's also a concern for many users, where the policy is strict about (un-annotated)
warnings. So, yeah, I guess its doable, but I'm not sure it worth the annoyances it causes
for users who just upgrade to a new FreeMarker version under an existing projects. And we
have a lot of users with "old" code bases.

For the "FreeMarker 3" activity though (if it will be started and lead to anywhere...) I will
most certainly want to support fluent API-s. And then things will be designed for that from
the beginning.

BTW, you have reported this for 2.3.23, which wasn't even required Java 5. Note that the latest
stable version is 2.3.25. I have changed the version to that.


was (Author: ddekany):
It is a builder all right, it just doesn't provide a fluent API. I know it's common for builders
nowadays, but having a fluent API is not the real point of using builders (like you had them
before Java 5 as well), that's a an extra for aesthetics. Anyway, then your point is that
should provide a fluent API. Therefore I will change this from a bug to RFE, and I hope you
don't mind.

The setters can't have a return value, because that conflicts with the JavaBeans specification.
But fluent API-s usually use {{foo(value)}} instead of {{setFoo(value)}} anyway, so luckily
both can exist on the same time. So no problem so far.

To add those {{foo(value)}} methods, we need the usual self-type generics hack, as you mention.
When these things were added, Java 5 wasn't a requirement, so the fluent API thing was out
of question. Now I guess they could be added, though frankly, I don't think users spare much
time with it... how often does one configure FreeMarker? And then, the same need will arise
with the {{Configuration}} API as well (and for all the others). We also have to consider
backward compatibility constraints, though at the first glance we only generate raw type warnings
here. But that's also a concern for many users, where the policy is strict about (un-annotated)
warnings. So, yeah, I guess its doable, but I'm not sure it worth the annoyances it causes
for users who just upgrade to a new FreeMarker version under an existing projects. And we
have a lot of users with "old" code bases.

For the "FreeMarker 3" activity though (if it will be started and lead to anywhere...) I will
most certainly want to support fluent API-s. And then things will be designed for that from
the beginning.

> DefaultObjectWrapperBuilder isn't a Builder
> -------------------------------------------
>
>                 Key: FREEMARKER-39
>                 URL: https://issues.apache.org/jira/browse/FREEMARKER-39
>             Project: Apache Freemarker
>          Issue Type: Improvement
>          Components: engine
>    Affects Versions: 2.3.23
>            Reporter: Brian Pontarelli
>
> This might not be considered a bug, but I'm logging it as one rather than an improvement.
The class {{DefaultObjectWrapperBuilder}} is not actually a builder right now and it makes
using it inline impossible. 
> To make it a more standard Builder pattern, all of the setter methods should be updated
so that the return type is {{DefaultObjectWrapperBuilder}} and the method does a {{return
this;}} at the end. This will make method chaining possible and inline use also possible.
Since it uses inheritance extensively (you might want to consider unwinding this as well),
you'll need to use generics and return T. Here's the class definition so that T works:
> {code:title=DefaultObjectWrapperBuilder.java}
> public class DefaultObjectWrapperBuilder extends DefaultObjectWrapperConfiguration<DefaultObjectWrapperBuilder>
> {code}
> And the parent class is defined like this:
> {code:title=DefaultObjectWrapperConfiguration.java}
> public abstract class DefaultObjectWrapperConfiguration<T> extends BeansWrapperConfiguration<T>
{
> {code}
> That the final parent class is defined like this:
> {code:title=BeansWrapperConfiguration.java}
> public abstract class BeansWrapperConfiguration<T extends BeansWrapperConfiguration>
implements Cloneable
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message