2013/5/22 Steve Ebersole <[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]> 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]>> >>>> >>>> >>>> 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]>> 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:hibernate-dev@lists.** >>>> jboss.org <[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:hibernate-dev@lists.** >>>> jboss.org <[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
