> On 2014/06/16, at 23:30, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> 
> As it is now, the code supports both builders and factory methods.  If it was 
> left that way then things could be converted over time.  But I am concerned 
> about users. We currently document using factory methods so any user created 
> plugins are doing that. If we switch to builders I would expect the code that 
> supports factory methods would want to be removed so user plugins with 
> factory methods would have to be converted.

That's a good point, I hadn't thought of that. 
> 
> Ralph
> 
>> On Jun 16, 2014, at 7:15 AM, Gary Gregory <garydgreg...@gmail.com> wrote:
>> 
>> FWIW, it does seem like a *lot* of work to redo some if not all plugins and 
>> I do understand Matt's POV. Would it even be possible to add builders later? 
>> Would we have to keep factory methods around for BC? I do not think there is 
>> a right and wrong pattern here, it's just that we have something that works 
>> now, indeed, with some pains here and there (default values for example).
>> 
>> Here is an alternate question: if we could wave a magic wand and get the 
>> work done now, which solution would we want? 
>> 
>> Gary
>> 
>> 
>> 
>>> On Mon, Jun 16, 2014 at 9:41 AM, Remko Popma <remko.po...@gmail.com> wrote:
>>> Matt,
>>> 
>>> Ralph's argument (and I agree) was that we want only one way to do plugins, 
>>> either with a factory method or with a builder.
>>> 
>>> Currently only two plugins use the new builder: PatternLayout and 
>>> HtmlLayout.
>>> So we can either revert these two back to use a factory method, or we can 
>>> convert the remaining 147 uses of the factory method to builders.
>>> That last option would mean writing the builders for those plugins, and 
>>> modifying all JUnit tests that currently use the factory methods.
>>> 
>>> I agree with Ralph that converting all remaining 147 places to use builders 
>>> would take up a lot of our time and energy for very little gain.
>>> 
>>> Matt, you mentioned that you could live with factory methods if there was a 
>>> better way to write JUnit tests than
>>> FileAppender.createAppender("true", "true", "true", "true", "true", "true", 
>>> "true", "4096", null, null, "false", null, null);
>>> 
>>> I believe I've demonstrated several ways in which such JUnit test code can 
>>> be improved.
>>> Are you okay with removing the builders and reverting back to the factory 
>>> methods?
>>> 
>>> 
>>> 
>>>> On Mon, Jun 16, 2014 at 10:23 PM, Matt Sicker <boa...@gmail.com> wrote:
>>>> I'm unable to load that page for some reason, but yes, using a factory 
>>>> method as such is really not extensible. If the factory method took a Map 
>>>> or some sort of configuration object (which itself could use the fluent 
>>>> builder pattern), that would be less tightly coupled.
>>>> 
>>>> Wouldn't it also make sense for the factory methods to be private or 
>>>> similar? That way they're only accessible through reflection.
>>>> 
>>>> 
>>>>> On Sunday, 15 June 2014, Paul Benedict <pbened...@apache.org> wrote:
>>>>> You don't want factory methods that take umpteen arguments. That's no way 
>>>>> to make your builder extensible for future enhancements. Instead, you 
>>>>> want a builder object that has a fluent API.
>>>>> 
>>>>> http://docs.spring.io/spring/docs/3.2.8.RELEASE/javadoc-api/org/springframework/beans/factory/support/BeanDefinitionBuilder.html
>>>>> 
>>>>> 
>>>>> Cheers,
>>>>> Paul
>>>>> 
>>>>> 
>>>>> On Sun, Jun 15, 2014 at 10:04 PM, Ralph Goers 
>>>>> <ralph.go...@dslextreme.com> wrote:
>>>>> Matt,
>>>>> 
>>>>> The only objection I have to builders is that there should only be one 
>>>>> way to configure plugins and I have neither the time or energy to convert 
>>>>> all plugins from factories to builders. With 130+ open issues I think our 
>>>>> time is better focused there instead of fixing something that already 
>>>>> works.
>>>>> 
>>>>> And, FWIW, the only place you will see createAppender coded like that is 
>>>>> in unit tests and there are a bunch of ways to make that clearer.
>>>>> 
>>>>> Ralph
>>>>> 
>>>>>> On Jun 15, 2014, at 7:19 PM, Matt Sicker <boa...@gmail.com> wrote:
>>>>>> 
>>>>>> I'm really against using factory methods due to language limitations in 
>>>>>> Java. You can't specify default values, for one. Two, the more 
>>>>>> parameters a factory takes, the crazier the method is. Seriously, tell 
>>>>>> me what this method is specifying:
>>>>>> 
>>>>>> FileAppender.createAppender("true", "true", "true", "true", "true", 
>>>>>> "true", "true", "4096", null, null, "false", null, null);
>>>>>> 
>>>>>> 
>>>>>> On 15 June 2014 21:05, Remko Popma <remko.po...@gmail.com> wrote:
>>>>>> I'm fine with just the factory methods too. 
>>>>>> 
>>>>>> Sent from my iPhone
>>>>>> 
>>>>>>> On 2014/06/16, at 9:44, Scott Deboy <scott.de...@gmail.com> wrote:
>>>>>>> 
>>>>>>> +1
>>>>>>> 
>>>>>>> On Jun 15, 2014 4:05 PM, "Ralph Goers" <ralph.go...@dslextreme.com> 
>>>>>>> wrote:
>>>>>>> Do we need the builders?  As I said, I prefer only one way for creating 
>>>>>>> plugins.
>>>>>>> 
>>>>>>> Ralph
>>>>>>> 
>>>>>>>> On Jun 15, 2014, at 2:49 PM, Remko Popma <remko.po...@gmail.com> wrote:
>>>>>>>> 
>>>>>>>> I see. I agree that the original format is much nicer. 
>>>>>>>> 
>>>>>>>> Matt, do you think you can achieve this with the builders?
>>>>>>>> 
>>>>>>>> Sent from my iPhone
>>>>>>>> 
>>>>>>>>> On 2014/06/16, at 1:29, Ralph Goers <ralph.go...@dslextreme.com> 
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> While you improved some of the existing messages, you really didm’t 
>>>>>>>>> address what I wanted fixed. The previous debug logs would have had 
>>>>>>>>> messages similar to:
>>>>>>>>> 
>>>>>>>>> Calling createLayout on class 
>>>>>>>>> org.apache.logging.log4j.core.layout.PatternLayout for element 
>>>>>>>>> PatternLayout with params(pattern="%d{HH:mm:ss.SSS} [%t] %-5level 
>>>>>>>>> %logger{36} - %msg%n", 
>>>>>>>>> Configuration(D:\rista\eclipsekws\.metadata\.plugins\org.eclipse.wst.server.core\tmp1\wtpwebapps\log4j2.0-test\WEB-INF\classes\test-log4j.xml),
>>>>>>>>>  null, charset="null", alwaysWriteExceptions="null")
>>>>>>>>> 
>>>>>>>>> Calling createAppender on class 
>>>>>>>>> org.apache.logging.log4j.core.appender.ConsoleAppender for element 
>>>>>>>>> Console with params(PatternLayout(%d{HH:mm:ss.SSS} [%t] %-5level 
>>>>>>>>> %logger{36} - %msg%n), null, target="SYSTEM_OUT", name="console", 
>>>>>>>>> follow="null", ignoreExceptions="null")
>>>>>>>>> 
>>>>>>>>> Calling createAppenderRef on class 
>>>>>>>>> org.apache.logging.log4j.core.config.AppenderRef for element 
>>>>>>>>> appender-ref with params(ref="console", level="null", null)
>>>> 
>>>> 
>>>> -- 
>>>> Matt Sicker <boa...@gmail.com>
>> 
>> 
>> 
>> -- 
>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org 
>> Java Persistence with Hibernate, Second Edition
>> JUnit in Action, Second Edition
>> Spring Batch in Action
>> Blog: http://garygregory.wordpress.com 
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
> 

Reply via email to