<div dir="ltr"><div>Hi,<br><br></div><div>I was recently doing a code review for a coworker and saw that he was concatenating strings and not using the varargs method of building the log message. I asked him why he was doing that and he explained it was because he also had a Throwable that he wanted to be passed as argument to see the stacktrace. He was not aware of the change in SLF4J 1.6.0 which allows us to pass a Throwable as last argument to the varargs.</div><div><br></div><div><br></div><div>"Unfortunately", when I was showing him implementations of this, I encountered the slf4j EventRecordingLogger and this implementation of the slf4j logger does not comply with this "new" behaviour. However I'm not sure if this is intentional. <br></div><div><br></div><div>Assuming it's not intentional, I've created a fork and I've made the required changes. But before I spend "days" on this while it's intentional I thought: let's write a quick email about this and ask for guidance.</div><div><br></div><div> I also saw that in the case of 2 Object arguments, the MessageFormatter seems to be suboptimal and is creating more objects then strictly required. <br></div><div><br></div><div>For the 1 Object argument case it it's also doing too much, certainly in case someone calls it with a Throwable. I've now added a runtime check to it. Since overloading it with a Throwable version will change the way the method behaves in case the caller does not know if it's being given a Throwable or an Object. It would be very illogical to call this method with a Throwable, with the 1.6 way of dealing with Throwables, since what do you want to format then?<br></div><div><br></div><div>But with the runtime check people won't loose their stacktraces, and otherwise in case the MessageFormatter is being used in a wrong way people will see their stacktraces being converted into one line because of a (minor? it's one instanceof call too many per call to the one Object logger variant) performance optimization.<br></div><div><br></div><div><br></div><div>I've not yet created the test cases for the EventRecordingLogger, I'll do that in case it's indeed unintentional that it's loosing the Throwables. I've added two tests for the changes I made to the MessageFormatter. While I was at it I also applied some improvements IntelliJ suggested.<br><br></div><div>You can see my changes on my fork:</div><div><br></div><div><a href="https://github.com/qos-ch/slf4j/compare/master...delgurth:eventrecordinglogger?expand=1">https://github.com/qos-ch/slf4j/compare/master...delgurth:eventrecordinglogger?expand=1</a></div><div><br></div><div>I hope you are able to review my changes and shed your light on it.<br></div><div><br></div><div>Kind regards,</div><div>Wessel van Norel<br></div></div>