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.
Why a singleton? A singleton is a creational design pattern whose purpose is to ensure that only one instance of a class exists. We want our logger to be a singleton as we only want one logger instance to be running and logging information at any one time.
It solves the problem of creating only one object of one class and that object can be used where ever required. Singleton is a creational pattern. This pattern ensures that there is only one instance of a class and provides a global access point for this object.
They violate the single responsibility principle: by virtue of the fact that they control their own creation and lifecycle. They inherently cause code to be tightly coupled. This makes faking them out under test rather difficult in many cases. They carry state around for the lifetime of the application.
The singleton design pattern solves problems by allowing it to: Ensure that a class only has one instance. Easily access the sole instance of a class. Control its instantiation.
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With