Which is a runtime error, not a compile time error.
Ralph
On Jun 16, 2014, at 7:27 AM, Paul Benedict <pbened...@apache.org> wrote:
> Of course. Yes, create() is supposed to validate.
>
>
> Cheers,
> Paul
>
>
> On Mon, Jun 16, 2014 at 9:26 AM, Gary Gregory <garydgreg...@gmail.com> wrote:
> On Mon, Jun 16, 2014 at 10:23 AM, Remko Popma <remko.po...@gmail.com> wrote:
> Gary, good question.
> To be honest, I'm not that convinced that builders are by definition better.
> For example, if you miss a parameter with the factory method, you get a
> compilation error.
> If you forget to call one of the builder.setValue(value) methods, you may
> never notice...
>
> Presumably, the builders would/should validate on build()/create(), unless
> the ctor of the target object already does that, which it should as well?
>
> Gary
>
> I see the argument that builders give names to the arguments, and so if you
> have many null arguments the builder code is still readable, but I already
> demonstrated that there are other ways to achieve the same readability.
>
> I'm thinking this is just a matter of style and personal preference.
>
>
>
> On Mon, Jun 16, 2014 at 11:15 PM, 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
>
>
>
>
> --
> 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
>
