views:

4387

answers:

14
public static Logger getLogger() {
 final Throwable t = new Throwable();
 final StackTraceElement methodCaller = t.getStackTrace()[1];
 final Logger logger = Logger.getLogger(methodCaller.getClassName());
 logger.setLevel(ResourceManager.LOGLEVEL);
 return logger;
}

This method would return a logger that knows the class it's logging for. Any ideas against it?

+2  A: 

You could of course just use Log4J with the appropriate pattern layout:

For example, for the class name "org.apache.xyz.SomeClass", the pattern %C{1} will output "SomeClass".

http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/PatternLayout.html

iAn
Don't over look the WARNING the use of %C. It will impact your performance.
bmatthews68
Using the standard idiom will give you the same result from the appender name *without* the performance impact. It is also subject to the StackTrace format being parsable e.g. AS400 Java had a weird stacktrace, it was human readable but sufficiently different as to be unusable for this purpose.
Michael Rutherfurd
+2  A: 

I guess it adds a lot of overhead for every class. Every class has to be 'looked up'. You create new Throwable objects to do that... These throwables don't come for free.

Daan
Isn't logging an overhead anyway? ;)
java.is.for.desktop
A: 

Why not?

public static Logger getLogger(Object o) {
  final Logger logger = Logger.getLogger(o.getClass());
  logger.setLevel(ResourceManager.LOGLEVEL);
  return logger;
}

And then when you need a logger for a class:

getLogger(this).debug("Some log message")
Mario Ortegón
Because it is readable and predictable rather than magic of course! ;-)
Eelco
+2  A: 

I prefer creating a (static) Logger for each class (with it's explicit class name). I than use the logger as is.

A: 

This mechanism puts in a lot of extra effort at runtime.

If you use Eclipse as your IDE, consider using Log4e. This handy plugin will generate logger declarations for you using your favourite logging framework. A fraction more effort at coding time, but much less work at runtime.

Bill Michell
+2  A: 

For every class that you use this with, you're going to have to look up the Logger anyway, so you might as well just use a static Logger in those classes.

private static final Logger logger = Logger.getLogger(MyClass.class.getName());

Then you just reference that logger when you need to do your log messages. Your method does the same thing that the static Log4J Logger does already so why reinvent the wheel?

18Rabbit
A: 

Unless you really need your Logger to be static, you could use

final Logger logger = LoggerFactory.getLogger(getClass());
Asgeir S. Nilsen
+5  A: 

Creating a stack trace is a relatively slow operation. Your caller already knows what class and method it is in, so the effort is wasted. This aspect of your solution is inefficient.

Even if you use static class information, you should not fetch the Logger again for each message. From the author of Log4j,Ceki Gülcü:

The most common error in wrapper classes is the invocation of the Logger.getLogger method on each log request. This is guaranteed to wreak havoc on your application's performance. Really!!!

This is the conventional, efficient idiom for getting a Logger is during class initialization:

private static final Logger log = Logger.getLogger(MyClass.class);

Note that this gives you a separate Logger for each type in a hierarchy. If you come up with a method that invokes getClass() on an instance, you will see messages logged by a base type showing up under the subtype's logger. Maybe this is desirable in some cases, but I find it confusing (and I tend to favor composition over inheritance anyway).

Obviously, using the dynamic type via getClass() will require you to obtain the logger at least once per instance, rather than once per class like the recommended idiom using static type information.

erickson
Standard idiom's are standard for a reason. If you do something different make sure that the additional maintenence hit of having to understand the difference is worth the effort.
Michael Rutherfurd
+1  A: 

You don't need to create a new Throwable object. You can just call Thread.currentThread().getStackTrace()[1]

ykaganovich
Look at the source code of Thread. It actually creates an exception if the thread you call getStrackTrace on is the same as the current one. So it's even (very slightly) more expensive.
Eelco
+2  A: 

Assuming you are keeping static refs to the loggers, here's a standalone static singleton:

public class LoggerUtils extends SecurityManager
{
    public static Logger getLogger()
    {
     String className = new LoggerUtils().getClassName();
     Logger logger = Logger.getLogger(className);
     return logger;
    }

    private String getClassName()
    {
     return getClassContext()[2].getName();
    }
}

Usage is nice and clean:

Logger logger = LoggerUtils.getLogger();
Erich
Nice trick. I would suggest keeping around the SecurityManager though. When you create an instance of it, it goes and checks a permission each time. Also you would have to instantiate your LoggerUtils inside a doPrivileged() block because of that.
Trejkaz
A: 

Actually I have a use case for this. I'm working with a legacy codebase that used its own 'logging api'. While I'm in development mode I can use this mechanism to rewrite the logging functions in the log API (which don't preserve the class names of their callers) and get the classes calling the logging api. It gives me more info and an ability to find out where a particular message is coming from without scouring the code.

Jherico
+2  A: 

We actually have something quite similar in a LogUtils class. Yes, it's kind of icky, but the advantages are worth it as far as I'm concerned. We wanted to make sure we didn't have any overhead from it being repeatedly called though, so ours (somewhat hackily) ensures that it can ONLY be called from a static initializer context, a la:

private static final Logger LOG = LogUtils.loggerForThisClass();

It will fail if it's invoked from a normal method, or from an instance initializer (i.e. if the 'static' was left off above) to reduce the risk of performance overhead. The method is:

public static Logger loggerForThisClass() {
    // We use the third stack element; second is this method, first is .getStackTrace()
    StackTraceElement myCaller = Thread.currentThread().getStackTrace()[2];
    Assert.equal("<clinit>", myCaller.getMethodName());
    return Logger.getLogger(myCaller.getClassName());
}

Anyone who asks what advantage does this have over

= Logger.getLogger(MyClass.class);

has probably never had to deal with someone who copies and pastes that line from somewhere else and forgets to change the class name, leaving you dealing with a class which sends all its stuff to another logger.

Cowan
A: 

Then the best thing is mix of two .

public class LoggerUtil {

    public static Level level=Level.ALL;

    public static java.util.logging.Logger getLogger() {
        final Throwable t = new Throwable();
        final StackTraceElement methodCaller = t.getStackTrace()[1];
        final java.util.logging.Logger logger = java.util.logging.Logger.getLogger(methodCaller.getClassName());
        logger.setLevel(level);

        return logger;
    }
}

And then in every class:

private static final Logger LOG = LoggerUtil.getLogger();

in code :

LOG.fine("debug that !...");

You get static logger that you can just copy&paste in every class and with no overhead ...

Alaa

Alaa Murad
A: 

But Making the variable static will not garbage the class object.

Anantn