I see similar code repetitions at work in projects. One of the things which I come up frequently nowadays is while using log4j logger:
void insertEmployee(Employee employee) {
// code …
if (logger.isDebugEnabled()) {
logger.debug(“Some log message”);
}
// code …
}
These lines are written to many places in code which makes the reading of it harder for the outcomer. Extract Method refactoring fits very good for such cases. The above lines can be moved to a method:
void writeLog(String logMessage) {
if (logger.isDebugEnabled()) {
logger.debug(logMessage);
}
}
and whenever a log meesage is needed, our new method can be called which will help the readers understand the code more easily and also keep it clean.
void insertEmployee(Employee employee) {
// code …
writeLog(“Some log message”);
// code …
}
In future if a need rise to change the log level, it can be easily done by updating our new method instead of searching and replacing every piece of code doing a logging.
Ruben said
The point of checking for isDebugEnabled() is avoiding the building of the String object (if this is a complex one with a lof of appends).
Adding a new method doesn’t help (the String will be built anyway, so you don’t win anything)
Cimballi said
And what happens if you need to log at different levels ? You should at least propose the five methods logDebug, logInfo, logWarn, …
serverdude said
Well, it may be read more easily, but it performs a lot worse unless you have constant strings.
The original version skips string building which is slow, unless the string is really needed because logging is enabled. Thus the original version is a har to read optimized version of the refactored version.