Common helper class for logging

Question

I have few classes which used System.out.println (which should be avoided) and now I am trying to move those into a logger.

We already have been using logger in some other modules. But the module I'm working haven't used it anywhere. So, I thought to create a new common class & put a new entry in logback.xml file for that new class.

<logger name="MyLogHelper" level="INFO" additivity="false">
 <!--appender-ref ref="STDOUT" /-->
 <appender-ref ref="LOG-FILE"/>
</logger>
<logger name="MyLogHelper" level="DEBUG" additivity="false">
 <!--appender-ref ref="STDOUT" /-->
 <appender-ref ref="LOG-FILE"/>
</logger>
<logger name="MyLogHelper" level="WARN" additivity="false">
 <!--appender-ref ref="STDOUT" /-->
 <appender-ref ref="LOG-FILE"/>
</logger>
<logger name="MyLogHelper" level="ERROR" additivity="false">
 <!--appender-ref ref="STDOUT" /-->
 <appender-ref ref="ERROR-FILE"/>
</logger>

We only maintain LOG-FILE and ERROR-FILE. Both are two appenders defined for files which are used in our system.

public class MyLogHelper
{
 private Logger logger = LoggerFactory.getLogger(MyLogHelper.class);
 private static MyLogHelper instance = new MyLogHelper();
 private MyLogHelper()
 {
 }
 public static MyLogHelper getInstance(){
 if(instance == null)
 instance = new MyLogHelper();
 return instance;
 }
 public void log(final String content, final LogLevel level)
 {
 switch (level)
 {
 case INFO:
 logger.info(content);
 break;
 case DEBUG:
 logger.debug(content);
 break;
 case WARN:
 logger.warn(content);
 break;
 case ERROR:
 logger.error(content);
 break;
 default:
 logger.debug(content);
 }
 }
 public enum LogLevel
 {
 INFO, DEBUG, WARN, ERROR
 }

And this is how I replace my System.out.println with my new way,

 /**Logger for logging within the class */
 private static final MyLogHelper logger = MyLogHelper.getInstance();
 ...
 logger.log(e.getMessage(), LogLevel.ERROR);

Is this a good design? Or can we improve this with a using a different design?

EDIT: I cannot simply replace the System.out.println with getting a logger and using it. Because in order to do that I have to add each class as entries to xml file.

What I actually looking for is, is my current design is scalable and my approach of using an enum and single pattern is the best choice for this situation?

• Robert Harvey: To evaluate whether this is a good design for you or not, you need to state your specific definition of "good." What qualities and features do you need in a logger that would qualify it as "good?"
• Jens: Why this helper class, if you want to Change the code, you can directly use a logging Framework.
• Vogel612: As outlined in the answer you already got: You're reinventing the wheel when you really shouldn't :/
• Martin Schröder: Please explain why you don't use @Log4J2 from Lombok with Log4j2.
• Walfrat: No, you don't need to add each class to the xml file. You can just configure the root level to "WARN" and everything will only print WARN level and above and so on. <root> <priority value="WARN" /> <appender-ref ref="LOG-FILE" /> <appender-ref ref="STDOUT" /> </root>

Answer 1

This

logger.error(e.getMessage());

is way more readable and faster to type than this :

logger.log(e.getMessage(), LogLevel.ERROR);

Furthermore : if you use a logger with your helper class your logs will all look like to this :

[MyLogHelper:XX] ERROR : blablalba // XX line of the switch case
[MyLogHelper:XX] WARN : foo

Instead of :

[FooClass:40] : ERROR : blablabla // logger.error in FooClass line 42
[BarClass:25] : WARN : foo

So not only is your code less readable, but you will lose precious information about where the log was triggered, which could be really annoying if you have the same message in different classes.

Finally don't do :

 logger.error(e.getMessage());

Do

logger.error("an unexpeced error happened while doing foooo", e);

So you will have the stacktrace print in your logs which is really precious information.