logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: debug output
Date Mon, 16 Jun 2014 15:11:22 GMT
Curious: how long do you think it would take you to convert all plugins?

Gary


On Mon, Jun 16, 2014 at 11:06 AM, Matt Sicker <boards@gmail.com> wrote:

> Paul, that's almost exactly what I was suggesting with the builders! I've
> only converted two so far as demos to see what everyone thought about doing
> it that way. If we wanted to wait until 2.1 or something to do a change
> like that, I'd be fine with that. I could spend the time converting them
> all which would also give me a nice overview of all the existing plugins.
>
>
> On 16 June 2014 08:49, Paul Benedict <pbenedict@apache.org> wrote:
>
>> I don't know why the Spring link isn't working (try later) but this is
>> how I think you guys can make the builder pattern better. Honestly, it's
>> impossible to comprehend what the parameters mean just by looking at the
>> code in the current form.
>>
>> Example returning a custom builder interface:
>> FileAppender.builder()
>>   .setPropertyX(true)
>>   .setPropertyY(true)
>>   .setPropertyZ(4096)
>>   .setPropertyA(null)
>>   .create();
>>
>> Paul
>>
>>
>>
>> Cheers,
>> Paul
>>
>>
>> On Mon, Jun 16, 2014 at 8:41 AM, Remko Popma <remko.popma@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 <boards@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 <pbenedict@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.goers@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 <boards@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.popma@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.deboy@gmail.com> wrote:
>>>>>
>>>>> +1
>>>>> On Jun 15, 2014 4:05 PM, "Ralph Goers" <ralph.goers@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.popma@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.goers@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 <boards@gmail.com>
>>>>
>>>
>>>
>>
>
>
> --
> Matt Sicker <boards@gmail.com>
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Mime
View raw message