Both really. You are adding a runtime performance hit to offset a possible development-time error. To me (if this is a real problem y'all have had to deal with), I'd rather see a build time check (checkstyle/findbugs rule maybe) that validates this by making sure that obtaining a logger is not passing a class other than the calling class.
On Wed 22 May 2013 08:42:37 AM CDT, Gunnar Morling wrote: > 2013/5/22 Steve Ebersole <[email protected] > <mailto:[email protected]>> > > I am not a fan of what goes on inside LoggerFactory.make(). > That's just my opinion. > > > Why is this? > > Which approach do you mean, the one creating a stack trace or the > other one using SecurityManager#getClassContext()? Or both :) > > > > > On Wed 22 May 2013 08:05:51 AM CDT, Sanne Grinovero wrote: > > Let's simplify my reply to address verbosity only then: > Search does: > > private static final Log log = LoggerFactory.make(); > > Looks good? > > Sanne > > On 22 May 2013 13:57, Steve Ebersole <[email protected] > <mailto:[email protected]>> wrote: > > Y'all are trying to solve a different problem imo. > > The "problem" I am looking to solve is simply verbosity. > Both the problem > and the solution have an equal possibility for copy-paste > errors. > > Whereas y'all are trying to remove the possibility of > these copy-paste > problems. BTW, I use IntelliJ IDEA "Live templates" to > deal with that. I > have a template named log; so I simply type "log"+TAB and > get a proper log > statement. > > > On Wed 22 May 2013 02:51:03 AM CDT, Gunnar Morling wrote: > > > 2013/5/21 Sanne Grinovero <[email protected] > <mailto:[email protected]> > <mailto:[email protected] <mailto:[email protected]>>> > > > Have you seen how Search does it? > > private static final Log log = > LoggerFactory.make(); > > The implementation of LoggerFactory#make is: > > public static Log make() { > Throwable t = new Throwable(); > StackTraceElement directCaller = > t.getStackTrace()[1]; > return Logger.getMessageLogger( Log.class, > directCaller.getClassName() ); > } > > We introduced this a while back after having > spotted some copy/paste > mistakes which had lead to have the wrong logger; > however there is a > catch: > each class initialization triggers a stacktrace > initialization. Sure > it's an initialization cost only, but still I > wonder how large the > cost is, adding up all classes: maybe we should > just replace it with a > checkstyle rule to verify its correctness. > > > An alternative implementation of LoggerFactory could > be this (based on > [1]): > > public final class LoggerFactory { > > private static CallerProvider callerProvider = new > CallerProvider(); > > public static Log make() { > return Logger.getMessageLogger( Log.class, > callerProvider.getCallerClass(__).getCanonicalName() ); > } > > private static class CallerProvider extends > SecurityManager { > public Class<?> getCallerClass() { > return getClassContext()[2]; > } > } > > SecurityManager#__getClassContext() is a native > method, so one doesn't > know how it is implemented, but I guess it's faster > than initializing > a stack trace, while still allowing for the concise > LoggerFactory.make(); usage. > > Btw. another copy-and-paste safe pattern enabled by > Java 7 is this > (suggested in "The Well-Grounded Java Developer"): > > Logger logger = Logger.getMessageLogger( > Log.class, > MethodHandles.lookup().__lookupClass().__getCanonicalName() > ); > > This can't be pushed into a factory class, though, > making more verbose > than the factory approach. > > --Gunnar > > [1] > > > http://beust.com/weblog/2011/__07/15/accessing-the-class-__object-in-a-static-context/ > > <http://beust.com/weblog/2011/07/15/accessing-the-class-object-in-a-static-context/> > > > Also, each module needs to have its own copy of > the LoggerFactory to > hardwire the correct Log.class interface, so you > could still import > the LoggerFactory from an alien module by > mistake, but that's likely > spotted by the typesafety of it as you wouldn't > have the expected > logger methods. > > Sanne > > > On 21 May 2013 22:24, Steve Ebersole > <[email protected] <mailto:[email protected]> > <mailto:[email protected] > <mailto:[email protected]>>> wrote: > > Forgot... So really this just allows more > conciseness in > obtaining the > > logger. So from: > > > > private static final CoreMessageLogger LOG = > Logger.getMessageLogger( > > CoreMessageLogger.class, > CollectionLoadContext.class.__getName() ); > > > > to: > > > > private static final CoreMessageLogger LOG = > CoreLogging.messageLogger( > > CollectionLoadContext.class ); > > > > > > On 05/21/2013 04:21 PM, Steve Ebersole wrote: > >> I was getting tired of statements in the > source code to get logger > >> instances that spread across sometimes 4 lines > because of JBoss > >> Logging's verbose means of acquiring a message > logger. So I > created a > >> more concise form for this for hibernate-core, > hibernate-entitymanager > >> and hibernate-envers. I mainly limited it to > these projects > because > >> they have lots of these calls, whereas the > others do not. Feel > free > >> to copy the approach to the other projects if > someone wants. > >> > >> Essentially each of those projects define a > class with 4 static > >> methods. Taking the hibernate-core one as an > example: > >> > >> > >> import org.jboss.logging.Logger; > >> > >> /** > >> * Quite sad, really, when you need helpers > for generating > loggers... > >> * > >> * @author Steve Ebersole > >> */ > >> public class CoreLogging { > >> /** > >> * Disallow instantiation > >> */ > >> private CoreLogging() { > >> } > >> > >> public static CoreMessageLogger > messageLogger(Class > >> classNeedingLogging) { > >> return messageLogger( > classNeedingLogging.getName() ); > >> } > >> > >> public static CoreMessageLogger > messageLogger(String > loggerName) { > >> return Logger.getMessageLogger( > CoreMessageLogger.class, > >> loggerName ); > >> } > >> > >> public static Logger logger(Class > classNeedingLogging) { > >> return Logger.getLogger( > classNeedingLogging ); > >> } > >> > >> public static Logger logger(String > loggerName) { > >> return Logger.getLogger( loggerName ); > >> } > >> } > >> > >> I just plan on replacing these calls as > opportunities arise, rather > >> than all in one fell swoop. > > > > _________________________________________________ > > hibernate-dev mailing list > > [email protected] > <mailto:[email protected]> > <mailto:hibernate-dev@lists.__jboss.org > <mailto:[email protected]>> > > > > https://lists.jboss.org/__mailman/listinfo/hibernate-dev > <https://lists.jboss.org/mailman/listinfo/hibernate-dev> > _________________________________________________ > hibernate-dev mailing list > [email protected] > <mailto:[email protected]> > <mailto:hibernate-dev@lists.__jboss.org > <mailto:[email protected]>> > https://lists.jboss.org/__mailman/listinfo/hibernate-dev > <https://lists.jboss.org/mailman/listinfo/hibernate-dev> > > > > _______________________________________________ hibernate-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/hibernate-dev
