<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Hi Ralph,</div><br><div><div>On 28.01.2010, at 08:42, Ralph Goers wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>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.<br></div></blockquote><div><br></div><div>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).</div><div>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).</div><br><blockquote type="cite"><div><br>On Jan 25, 2010, at 3:40 AM, Joern Huxhorn wrote:<br><br><blockquote type="cite">Ok, I've had time to take a look now.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">My plan was that the logback-classic Logger would simply extend org.slf4j.n.helper.AbstractLoggerBase.<br></blockquote><br>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 <br></div></blockquote><div><br></div><div>But it does exist:</div><div><a href="http://github.com/huxi/slf4j/blob/slf4j-redesign/slf4j-n-api/src/main/java/org/slf4j/n/helpers/AbstractLoggerBase.java">http://github.com/huxi/slf4j/blob/slf4j-redesign/slf4j-n-api/src/main/java/org/slf4j/n/helpers/AbstractLoggerBase.java</a></div><div><br></div><div>Is it possible that you did not look at the slf4j-redesign branch but master?</div><div><br></div><div>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.</div><div>It also supports proper deserialization of Logger instances already.</div><div><br></div><div>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.</div><br><blockquote type="cite"><div><br><blockquote type="cite"><br></blockquote><blockquote type="cite">That way, only the following 3 methods would need implementation:<br></blockquote><blockquote type="cite">Threshold getThreshold()<br></blockquote><blockquote type="cite">boolean isEnabled(Level level, Marker marker)<br></blockquote><blockquote type="cite">void log(Level level, Marker marker, Message message, Throwable throwable)<br></blockquote><br>That isn't really true. Again, look at the Logger class. While the public methods like info(), debug() etc, could be factored out there are a lot of other methods that can't be in an abstract class.<br></div></blockquote><div><br></div><div>Those would stay in the actual Logger implementation, unchanged, as before.</div><br><blockquote type="cite"><div><br><blockquote type="cite"><br></blockquote><blockquote type="cite">The log-method above would just call filterAndLog(String localFQCN, Marker marker, Level level, Message msg, Throwable t).<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">The implementation of old and new SLF4J-API implementations must be separated so it's possible to either call org.slf4j.n.LoggerFactory (for org.slf4j.n.Logger) or org.slf4j.LoggerFactory (for org.slf4j.Logger) while deserializing a Logger, i.e. the Logback Logger implementation should *not* implement org.slf4j.Logger anymore.<br></blockquote><br>There are some issues with this. LoggerFactory returns a Logger. To maintain compatibility this interface cannot be changed. Instead I created MessageLoggerFactory which returns a MessageLogger which extends LocationAwareLogger. Logback's Logger now implements MessageLogger instead of LocationAwareLogger. Other applications, such as Sling, which implement the Logger interface will not be impacted by this at all. I don't see how using org.slf4j.message.MessageLoggerFactory is much different than using org.slf4j.new.LoggerFactory, except I believe my name is a bit clearer.<br><br></div></blockquote><div><br></div><div>The original Logger interface is unchanged and would stay that way. No other implementation would be affected at all since they would be automatically wrapped the other way around, i.e. there's a n.ILoggerFactory implementation already implemented that is used automatically if no explicit implementation is available.</div><div>It wraps an original Logger into the n.Logger interface.</div><div><br></div><div>Both scenarios are tested in</div><div><a href="http://github.com/huxi/slf4j/tree/slf4j-redesign/slf4j-n-api-new-integration-test/">http://github.com/huxi/slf4j/tree/slf4j-redesign/slf4j-n-api-new-integration-test/</a></div><div>and</div><div><a href="http://github.com/huxi/slf4j/tree/slf4j-redesign/slf4j-n-api-old-integration-test/">http://github.com/huxi/slf4j/tree/slf4j-redesign/slf4j-n-api-old-integration-test/</a></div><br><blockquote type="cite"><div>Logback doesn't implement Logger directly in either Ceki's fork or mine. Currently, it implements LocationAwareLogger. In mine, MessageLogger. <br></div></blockquote><div><br></div><div>There's an additional (optional) interface for that, too:</div><div><a href="http://github.com/huxi/slf4j/blob/slf4j-redesign/slf4j-n-api/src/main/java/org/slf4j/n/spi/LocationAwareLogger.java">http://github.com/huxi/slf4j/blob/slf4j-redesign/slf4j-n-api/src/main/java/org/slf4j/n/spi/LocationAwareLogger.java</a></div><br><blockquote type="cite"><div><br><blockquote type="cite"><br></blockquote><blockquote type="cite">Instead, org.slf4j.impl.StaticLoggerBinder should return org.slf4j.n.helpers.UsingNewLoggerFactory which is implemented like this:<br></blockquote><blockquote type="cite">Logger getLogger(String name) {<br></blockquote><blockquote type="cite"> return org.slf4j.n.LoggerFactory.getLogger(name).getOldLogger();<br></blockquote><blockquote type="cite"> }<br></blockquote><br>Why? I left the existing getLogger alone. Instead the MessageLogger is a LoggerWrapper, so the existing API is untouched.<br></div></blockquote><div><br></div><div>Because Logger and n.Logger are compiletime- but not binary-compatible.</div><div><br></div><div>Assuming that more and more modules would switch to the n API it means that more and more Loggers are used without an additional wrapper.</div><div><br></div><div>The current LoggerFactory-related code would be moved to org.slf4j.n.impl.StaticLoggerBinder and an org.slf4j.n.ILoggerFactory implementation with very minimal changes.</div><div><br></div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000"><br></font><blockquote type="cite"><br></blockquote><blockquote type="cite">Does this help as an explanation so far?<br></blockquote><br>Not really. I'm working under the premise that a) SLF4J is not being upgraded to Java 5</div></blockquote><div><br></div><div>Yes, only an additional, optional module would be added. The original SLF4J API would not require Java 5.</div><br><blockquote type="cite"><div> b) no new SLF4J version is required because c) backward compatibility must be maintained.</div></blockquote><div><br></div><div>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.</div><div>My constraining goal was compiletime compatibility, but not binary compatibility.</div><div><br></div><div>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.</div><div>So I aimed for the next-best thing: compiletime compatibility requiring only a change of the Logger and LoggerFactory import.</div><div><br></div><div>All modules using the original API could still do so while other that use JDK>=1.5 could opt-in using the "modern" API by simply changing the import.</div><div><br></div><div>I've put a lot of thought into that whole concept.</div><div><br></div><div>One benefit of this approach is that it does not require the user to learn anything new. If he's not interested in the new API he'll be perfectly fine ignoring it entirely.</div><div>If, on the other hand, he has the need and/or motivation for the new Message/Varargs logging then a very simple upgrade path is in existence.</div><div>It can't get any simpler than just adding a .n to the imports (given the constraint that the original API must be left unchanged).</div><div><br></div><div>The .n is just a suggestion, though.</div><div>I really don't care how that package should be named or if everything is put into a new namespace like org.lf4j.</div><div><br></div><div>What I tried to achieve is that the class-names don't have to be changed, just the import/package-name.</div><br><blockquote type="cite"><div> 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. </div></blockquote><div><br></div><div>It wouldn't break anything. I tested it.</div><div><br></div><div>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.</div><div><br></div><div>For example:</div><div>during the evolution of SLF4J bug #31 I suggested that n.Logger should extend Logger.</div><div>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. </div><div><br></div></div>Cheers,<div>Joern.</div><div><br></div></body></html>