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 <steve(a)hibernate.org
<mailto:steve@hibernate.org>>
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 <steve(a)hibernate.org
<mailto:steve@hibernate.org>> 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 <sanne(a)hibernate.org
<mailto:sanne@hibernate.org>
<mailto:sanne@hibernate.org
<mailto:sanne@hibernate.org>>>
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-st...
<
http://beust.com/weblog/2011/07/15/accessing-the-class-object-in-a-static...
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
<steve(a)hibernate.org <mailto:steve@hibernate.org>
<mailto:steve@hibernate.org
<mailto:steve@hibernate.org>>> 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
> hibernate-dev(a)lists.jboss.org
<mailto:hibernate-dev@lists.jboss.org>
<mailto:hibernate-dev@lists.__jboss.org
<mailto:hibernate-dev@lists.jboss.org>>
>
https://lists.jboss.org/__mailman/listinfo/hibernate-dev
<
https://lists.jboss.org/mailman/listinfo/hibernate-dev>
_________________________________________________
hibernate-dev mailing list
hibernate-dev(a)lists.jboss.org
<mailto:hibernate-dev@lists.jboss.org>
<mailto:hibernate-dev@lists.__jboss.org
<mailto:hibernate-dev@lists.jboss.org>>
https://lists.jboss.org/__mailman/listinfo/hibernate-dev
<
https://lists.jboss.org/mailman/listinfo/hibernate-dev>