1

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?

  • 2
    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?" – Robert Harvey Aug 23 '16 at 6:09
  • Why this helper class, if you want to Change the code, you can directly use a logging Framework. – Jens Aug 23 '16 at 6:13
  • 2
    As outlined in the answer you already got: You're reinventing the wheel when you really shouldn't :/ – Vogel612 Aug 23 '16 at 10:57
  • Please explain why you don't use @Log4J2 from Lombok with Log4j2. – Reinstate Monica - M. Schröder Aug 23 '16 at 21:55
  • 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> – Walfrat Aug 24 '16 at 6:33
5

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.