On 09.02.2009, at 23:15, Ceki Gulcu wrote:
Hello Joern,
At an earlier time, certain classes expected the Appender interface
to have a setter/getter for the layout property.
AbstractLayoutAction is one such class. However, it is no longer
used. (I just removed it.)
Oh, I see, "for historic reasons" :)
Many logback appenders do not require a layout and can have their
layout property set to null. Having a layout property in Appender
and AppenderBase is just noise for such appenders but otherwise not
harmful. They have a property, i.e. layout, which they don't use...
Well, yes, you are right. It's not downright harmful... but it is
somewhat confusing for users (not developers) of the appenders and
makes them wonder what the layout is about. People actually asked me
about that...
If the Appender interface did not contain the layout property, we
would probably have two distinct appender base implementations,
AppenderBase and AppenderBaseWithLayout (possibly with an additional
interface such as AppenderWithLayout). Come to think of it, we would
also need to handle the UnsyncronizedAppenderBase branch of the
class hierarchy. Adding UnsyncronizedAppenderBaseWithLayout would
be too unwieldy. :-)
I don't think that this would be necessary because it would be enough
if the mentioned classes would simply implement Appender (without the
layout methods) and appenders requiring the layout would additionally
implement the extended interface, in just the way that is necessary
for their desired behavior (e.g. even synchronized/locked if required,
which isn't the case right now).
This is more or less an aesthetic reasoning and I wouldn't consider it
very important.
Concerning your original question: the new implementation is
definitely better than before!
For developers coming from log4j, AppenderBase having the same
implementation for setLayout and getLayout as log4j's
AppenderSkeleton class would help them migrate to logback with a
little bit more ease, at least with less surprise.
I wonder what I was thinking when I implemented layout setter and
getters in LayoutBase as nop.
I know this feeling all too well...
But this is actually a good thing because it's a sign that we keep
developing our skills ;)
Joern.
Joern Huxhorn wrote:
Hi Ceki,
the only thing that I don't understand is why Appender requires a
layout at all.
It would by cleaner if there was a sub-interface, e.g.
LayoutAwareAooender (just a spontaneous suggestion), that extended
Appender and would add said methods.
Some appenders, like SocketAppender or some fictitious appender
that would simply serialize the events to a file (I'm planning to
implement such an appender, btw), just don't need a layout at all.
AppenderBase would then just implement the basic Appender
interface, leaving the layout implementation to appenders that
would really require it.
This topic is covered by both http://jira.qos.ch/browse/LBCORE-1
and http://jira.qos.ch/browse/LBCORE-56
Regards,
Joern.
--
Ceki Gülcü
Logback: The reliable, generic, fast and flexible logging framework
for Java.
http://logback.qos.ch
_______________________________________________
logback-dev mailing list
[email protected]
http://qos.ch/mailman/listinfo/logback-dev
_______________________________________________
logback-dev mailing list
[email protected]
http://qos.ch/mailman/listinfo/logback-dev
