Mailing-List: contact notifications-help@freemarker.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@freemarker.incubator.apache.org
Date: Sun, 6 Nov 2016 09:51:58 +0000 (UTC)
From: "Daniel Dekany (JIRA)" <jira@apache.org>
To: notifications@freemarker.incubator.apache.org
Message-ID: <JIRA.13018631.1478368705000.202704.1478425918332@Atlassian.JIRA>
In-Reply-To: <JIRA.13018631.1478368705000@Atlassian.JIRA>
References: <JIRA.13018631.1478368705000@Atlassian.JIRA> <JIRA.13018631.1478368705760@arcas>
Subject: [jira] [Comment Edited] (FREEMARKER-39) DefaultObjectWrapperBuilder
 isn't a Builder
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Sun, 06 Nov 2016 09:52:04 -0000


    [ 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)