Hi Ralph,

On 28.01.2010, at 08:42, Ralph Goers wrote:

In reading your response I'm not sure if you are just describing how you originally thought this should be done or critiquing what I've done. In your analysis below I really don't see any comments about what is wrong but rather, how you had anticipated it would be done. I've tried to answer the points in your email as best I can, but the main issue is that it seemed to me that your proposal was for a new version of SLF4J whereas I am trying to make the changes without the need for a major release or breaking any compatibility.

I assumed you tried to do the Logback counterpart of my SLF4J redesign proposal - which wouldn't break compatibility but introduces an additional artifact (tentatively called slf4j-n-api).

Instead of depending on slf4j-api, Logback classic would instead depend on slf4j-n-api (which, in turn, introduces the slf4j-api dependency for compatibility reasons).

I tried to implement your proposal to some degree. The most important part to me was introducing the Message concept. For minimal impact on users I did not want to introduce or change any dependencies, other than requiring version changes. 

On Jan 25, 2010, at 3:40 AM, Joern Huxhorn wrote:

Ok, I've had time to take a look now.

My plan was that the logback-classic Logger would simply extend org.slf4j.n.helper.AbstractLoggerBase.

I saw that in your original note. That class doesn't currently exist. I assume you would have created it. As I recall I thought about that but decided against it. Logback has a lot of logic in its Logger class and implementing AbstractLoggerBase in SLF4J doesn't save much as you might think.  In addition, all the current Loggers implement the interface. Creating a new base class that includes the Message API and then having Logback extend that creates serious problems

But it does exist:

http://github.com/huxi/slf4j/blob/slf4j-redesign/slf4j-n-api/src/main/java/org/slf4j/n/helpers/AbstractLoggerBase.java

Is it possible that you did not look at the slf4j-redesign branch but master?

Sorry. It has been quite a while since I wrote the code. Yes, I looked at your code. IIRC the reason I didn't use it was that I don't like calling the isEnabled methods that way. The log method is going to result it Logback calling filterAndLog which results in filtering being performed twice for each call. If the log method doesn't call filterAndLog then the behavior of the log method would be different than how Logback currently behaves. Of course, not calling isEnabled would result in creating ParameterizedMessages when no logging will occur. My implementation doesn't have this problem.

The Logback classic Logger isn't extending anything at the moment and the AbstractLoggerBase is simply implementing everything that will likely stay the same for all Logger implementations.

It also supports proper deserialization of Logger instances already.

It's not necessary to extend it but it's certainly easier to do so and doing so wouldn't introduce any problems in case of the Logback classic Logger. It would simply remove a lot of boilerplate code from the Logger implementation.

That would be true if my comment above wasn't true.

b) no new SLF4J version is required because c) backward compatibility must be maintained.

That's the whole point. By leaving the original API untouched and adding an additional one we'd have the possibility to change the API without constraints.

My constraining goal was compiletime compatibility, but not binary compatibility.

Binary compatibility simply can't be achieved without sacrificing JDK<1.5 compatibility in SLF4J API - and I think that would be a bad idea.

So I aimed for the next-best thing: compiletime compatibility requiring only a change of the Logger and LoggerFactory import.

??? My code should be maintaining binary compatibility. Applications written to older versions of SLF4J can drop in the new jars with no change. New code can use the new API. They can coexist happily.

If I was not working with those constraints I certainly would have made a few different choices, but then I'm sure I would have had a snowballs chance in hell of getting it adopted since it would break so much stuff.  

It wouldn't break anything. I tested it.

I won't believe that until I see a fork of Logback that depends on your SLF4J implementation. It was very difficult to get Logback to accept the Message and deal with it correctly.

If you can find any breakage please let me know but I'm quite confident that I didn't miss any point this time around.

For example:

during the evolution of SLF4J bug #31 I suggested that n.Logger should extend Logger.

This would have resulted in breakage since it wouldn't have been clear, during deserialization, what is expected by the deserializing code. readResolve() needs to execute LoggerFactory.getLogger(loggerName). How should we decide about wrapping or not without creating kludgy code? Wrapping it each and every time if n.Logger wasn't implemented by the returned instance? Nope, this would have been quite bad. 

If you notice there really is no extra overhead since in my fork Logback's getLogger() always returns a MessageLogger. It isn't really wrapped. No kludgy code is needed. Since MessageLogger extends Logger anyone calling LoggerFactory.getLogger() will work just fine.  The wrapper is only needed if a Logger implementation returns a Logger. Then MessageLoggerFactory will wrap it.

Ralph