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: Pluggable message creation
Date Thu, 06 Dec 2012 22:02:58 GMT
On Thu, Dec 6, 2012 at 3:01 PM, Ralph Goers <ralph.goers@dslextreme.com>wrote:

>
> On Dec 6, 2012, at 11:40 AM, Gary Gregory wrote:
>
> Hi Ralph and all,
>
> On Thu, Dec 6, 2012 at 11:14 AM, Ralph Goers <ralph.goers@dslextreme.com>wrote:
>
>>
>> On Dec 6, 2012, at 6:04 AM, Gary Gregory wrote:
>>
>> I craeted https://issues.apache.org/jira/browse/LOG4J2-133 to save a
>> patch.
>>
>> I'd like to discuss it on the ML here.
>>
>> Thoughts?
>>
>>
>> Yeah, I have a few.
>>
>> First, I expected MessageFactory to be an interface, not a class.
>>
>
> OK, we can do that.
>
>
>>
>> Second, I expected to see 3 MessageFactory implementations to start that
>> would allow the user to choose between ParameterizedMessage,
>> StringFormatMessage and MessageFormatMessage (I have that in the works for
>> JUL support).
>>
>
> OK, we can have one factory with 3 methods, or three factories with one
> method each. I like your idea, but... that means the AbstractLogger needs
> to track three factories instead of one.
>
>
> I'm not sure I was clear. The 3 MessageFactory instances would be
> StringFormatMessageFactory, MessageFormatMessageFactory and
> ParameterizedMessageFactory.   The newMessage method that accepts a String
> and an Object array would use the corresponding Message implementation.
>  Each of them could still expose a newMessage taking an Object and a new
> Message taking just a String but those would be the same (so probably they
> would extend an abstract class).
>

What I am not crazy about with this approach is that my call sites now have
to look like:

Logger log = LogManager.getLogger(Foo.class, new ObjectMessageFactory(),
new StringMessageFactory(), new ParameterizedMessageFactory())

instead of:

Logger log = LogManager.getLogger(Foo.class, new MyMessageFactory())

The fine grain of the design translates to too much work on the user side.

It is still possible to use the short hand and finer design if we allow for
a MyMessageFactory as a holder for the three message factories. It seems to
get a little convoluted though.

Thoughts?

Gary


>
>
>>
>> Third, I expected that only the methods that accept a format and a
>> parameter would use the MessageFactory and SimpleMessage and ObjectMessage
>> would just be left as they are. That doesn't mean I couldn't be persuaded
>> to include them in the interface as I could see uses customizing that for
>> their needs.
>>
>
> KISS it for now, I could go for that, but it not as easy to explain as
> "you can plugin a factory",  as opposed to "you can plugin a factory for
> this kind of message but not for that kind".
>
>
> See my comment above.
>
>
>> Lastly, I'm not sure how adding getThrowable to the Message interface is
>> related or required for this change.  I'm not necessarily opposed but I'm
>> not sure if it should be tied to this.
>>
>
> Ah, that one, you have to see in code in AbstractLogger:
>
>     public void debug(String message, Object... params) {
>         if (isEnabled(Level.DEBUG, null, message, params)) {
>             Message msg = messageFactory.newMessage(message, params);
>             log(null, FQCN, Level.DEBUG, msg, *msg.getThrowable()*);
>         }
>     }
>
> Because AbstractLogger calls *msg.getThrowable()*, you need to be able to
> do that generically to keep it simple. But this does not have to be the
> case if ParameterizedMessage and StringFormattedMessage both implement a
> common interface that implements getThrowable(). This interface would
> extend Message and be called...? Or you can add getThrowable() to Message.
> Throughts?
>
>
> OK. This seems reasonable to me.
>
> Ralph
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
JUnit in Action, 2nd Ed: <http://goog_1249600977>http://bit.ly/ECvg0
Spring Batch in Action: <http://s.apache.org/HOq>http://bit.ly/bqpbCK
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Mime
View raw message