Benefits of Log4j singleton wrapper?

问题:

I have recently inherited some Java code and need to integrate it into a project that I am working on. My project is a service agent that processes and transforms XML messages. While looking through the new code, I discovered the following logging class:

import org.apache.log4j.BasicConfigurator;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;

public class MyLogger {

    private static MyLogger instance = null;
    protected final static Logger log = Logger.getLogger(MyLogger.class);

    private MyLogger() {
        super();
    }

    public static MyLogger getInstance(){
        if(instance  == null){
            instance  = new MyLogger();
            BasicConfigurator.configure();
            log.setLevel(Level.ALL);
        }
        return instance;
    }

    public void info(String myclass, String msg) {
        log.info("[" + myclass + "] " + msg);

    }

    public void error(String myclass, String msg, Exception ce) {               
        log.error("[" + myclass + "] " + msg, ce);      
    }

    public void warning(String myclass, String msg) {
        log.warn("[" + myclass + "] " + msg);
    }    
}

This class basically wraps log4j with (another) singleton. All of the logging in the classes that I need to integrate look something like this:

public class MyClass {
   private final static MyLogger log = MyLogger.getInstance();
   private final static String myclass = MyClass.class.getName();

   ...

   log.info(myclass, "Information message...");   
}

I do not see any obvious benefit to using an extra class for logging, thus I am considering refactoring this code to remove the MyLogger class and log in the following fashion:

import org.apache.log4j.Logger;

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

   ...

   log.info("Information Message...");     
}

This would make the logging mechanism consistent across the project. Before I do this, I would like to know if there are any benefits to wrapping Log4j with a singleton class that I may be missing. Thanks!

EDIT: Thanks everyone for the helpful answers - I pickup up several new insights from each. Accepted Nathan Hughes' answer for pointing out lost functionality by leaving the class intact - I had been assuming that the biggest downside to leaving the singleton alone was simply code bloat. I will trash the class.

回答1:

Get rid of it. Using this monstrosity means all logging that goes through it will be listed with the same logger (MyLogger) and method (which is why the arguments to its methods include the class of the thing being logged). That means not only do you have to add any class, method, and line number information to each logger call, but you can't do any filtering on log levels for different packages the way you could using the typical log4j approach with classes as loggers.

This thing is a piece of junk and you are better off without it.

回答2:

The only benefit that I could see is that it would be easy to swap out the log4j implementation with another logging implementation, or get the logging to do something much more customised such as log to one of your own databases.

That said, I would still refactor the code to use log4j directly. Or, more likely in my case, to use SLF4J.

回答3:

One thing your inherited code does that log4j does is make thing non-thread safe. Since there is no locking in getInstance() you can potentially hand out more than one instance and break the singleton intentions of the code.

You also lose the ability to set logging levels for each class depending on what you are doing.

回答4:

The only flaw I can tell is, because of this declaration:

protected final static Logger log = Logger.getLogger(MyLogger.class);

The logger is, essentially, hooked to object MyLogger and all logging information/errors/warnings, etc. will be "linked" to MyLogger. You have no idea which object added the logging information, none whatsoever.

The only advantage I see with this Singleton, is:

• Single instantation: You never have to worry of declaring a static final Logger implementation all the time.
• You don't have to worry what type of Logger you are using. One can change the type of Logger only in this Singleton class. Any further changes to logging is done only in the Singleton.

I have seen this in my company as well, but I don't suggest that type of setup. Rather use SLF4J or the Java Logging Framework.