Encapsulating logging and throwing exceptions into a method. Bad practice? original code example

if (var1 == null) {
    String msg = "var 1 is null";
    logger.log(msg);
    throw new CustomException(msg);
}
if (var2 == null) {
    String msg = "var 2 is null";
    logger.log(msg);
    throw new CustomException(msg);
}
if (var3 == null) {
    String msg = "var 3 is null";
    logger.log(msg);
    throw new CustomException(msg);
}
return var3.toString(); // ok


new code

if (var1 == null) {
    logMessageAndThrowException("var 1 is null");
}
if (var2 == null) {
    logMessageAndThrowException("var 2 is null");
}
if (var3 == null) {
    logMessageAndThrowException("var 3 is null");
}
return var3.toString(); // NetBeans warn me here for a NPE

void logMessageAndThrowException(String msg) throws CustomException {
    logger.log(msg);
    throw new CustomException(msg);
}


Is this a bad practice or NetBeans just are not able to understand the code?
The second version removes some duplication, so it's better, but usually you should choose one of logging and rethrowing. Frameworks/appservers usually catch and log uncatched exceptions so you will have two (or more) entries for the same exception it the log file which makes debugging hard.


  Log the exception only once!
  
  One of the common pain points that I have noticed is logging and
  rethrowing an exception. As a result, the log files contains the same
  exceptions several times on several stack levels.


Author: Nayaki