2

If I have a class that I want to accept an optional logger to log debug information:

public class A {

private Logger logger ; 

public A( Logger logger ) { 
    this.logger = logger ; 
}

public A() { 
    this( null ) ;
}

public void hello() {
    logger.debug( "hello" ) ;
}

public void goodbye() {
    logger.warn( "goodbye" ) ; 
}

}

Is there any way for me to avoid the need for constant null checks?

public void hello() {

    if( logger != null ) { 
        logger.debug( "hello" ) ;
    }

}

public void goodbye() {

    if( logger != null ) {
        logger.warn( "goodbye" ) ; 
    }

}

I thought maybe if I made a logger wrapper class, that could do the checks for me and then either do nothing, or log the message. But then I would need to wrap each method I would want to use.

3
  • Is it intended that logger can be set to null? Or are you just defensive coding? Is a null logger the way you want to "turn off" logging?
    – Bert F
    Feb 7, 2011 at 21:37
  • It's intended, yes; I would like to make an object that will output messages using a logger if one is provided, but don't output anything if there isn't one. Feb 7, 2011 at 21:41
  • again then make a custom impl, that has all its warn/info/debug methods totally empty.
    – bestsss
    Feb 7, 2011 at 21:44

7 Answers 7

9

I would create Logger as an interface and implement the real logger and a dummy class that doesn't log anything. This way, you can always just access the logger without any null checks.

4

All answers basically suggest using Null Object pattern, see:

2

2 things:

  1. always initialize the logger
  2. (and/or) use a sentinel logger that doesn't to anything at all
2

Something like this maybe:

private void logDebug(final String str, final Object ... parameters ) {
    if (this.logger != null && this.logger.isDebugEnabled()) {
        this.logger.debug(
           parameters.length > 0
              ? MessageFormat.format(str, parameters)
              : str
        );
    }
}

private void logInfo(final String str,final Object ... parameters) {
    if (this.logger != null && this.logger.isInfoEnabled()) {
        this.logger.info(
           parameters.length > 0
              ? MessageFormat.format(str, parameters)
              : str
        );
    }
}

Now you can just call logDebug(str) or logInfo(str), and you can also interpolate the log messages with parameters using MessageFormat.format():

logDebug("Called {0} method with parameters {1} and {2}","doStuff",foo,bar);
1

Sorry if it sounds dumb, but won't making the methods Warn and Debug static solve this ? If you need an instance just create it inside the static methods, or if its a singleton check forr null inside the static methods.

1
  • Exactly, the famous singleton pattern :-) Nov 16, 2012 at 12:10
0

You could set a boolean "enableLogging" to true in your 1-parameter constructor, and to false in your empty constructor. Thus you give more meaning to your checks - and they are faster.

0

The main problem is you are using null everywhere, which means naturally you need to test for null everywhere. Create a Logger where all methods do nothing and use that instance where appropriate.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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