How to not convert a Boolean to boolean

On my last code review session I stumbled over that lovely piece of code:

Boolean wrapper = foo();
boolean primitive = Boolean.parseBoolean(""+wrapper);

A somehow interesting way of converting a Boolean to it’s primitive.

I guess this was done to avoid a NullPointerException in case wrapper is null. But it violates one simple rule: write readable code.

Of course, this line isn’t hard to understand—but do you know for sure the outcome of the parseBoolean-method when the argument is null or an empty String? Is it ‘false’, ‘true’ or will the method throw an exception? To make it even worse: “”+wrapper will return the String “null” (yep – a String of length 4) when wrapper is null. Can parseBoolean handle this too?

So how to (really) avoid a NullPointer here? Simply use:

Boolean wrapper = foo();
boolean primitive = wrapper == null ? false : wrapper.booleanValue();

or (cooler but a little less readable):

Boolean wrapper = foo();
boolean primitive = wrapper != null && wrapper.booleanValue();

Logging Anti-Patterns, Part I

Recently I refactored a legacy application which based on a self-implemented logging-framework (I replaced the proprietary logging-code with slf4j). Along that I stumbled over a bunch of anti-patterns I want to discuss further on.

1. Logger-instantiation inside a method

In general there should be only one logger per class. Most of the common logging-frameworks are caching their loggers after instantiation. So calling a LoggerFactory inside a method is simply an unnecessary code and a (slight) performance waste. Just instantiate a static final logger in the class and use that one on every logging event.

Anti-pattern

public void foo() {
	...
	Logger logger = LoggerFactory.getLogger(Foo.class);
	logger.info("bar!");
	...
}

Pattern

private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

public void foo() {
	...
	LOGGER.info("Bar");
	...
}

2. Logger-instantiation using the exception message

This is one of my favorites: on each exception a new logger is created using the exception message as logger name. That neither makes sense nor does it help when analyzing the log-files. It also wastes resources: if the exception messages differ, every factory-call will create a new logger instance (instead of returning a cached one) and additionally adds the new logger to the factory’s cache. As a result the cache will be blown up with multiple logger instances—which won’t be used furthermore.

Anti-pattern

public void foo() {
	try {
	...
	} catch (Exception e) {
		Logger logger = LoggerFactory.getLogger(e.getMessage());
		logger.error(e.getMessage(), e);
	}
}

Pattern

private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

public void foo() {
	try {
		...
	} catch (Exception e) {
		LOGGER.error(e.getMessage(), e);
	}
}

3. Logger-instantiation using the enclosing class’ name as string

For ordinary loggers the factory-method of LoggerFactory should be called passing the enclosing class. Avoid to pass a class’ name as string. If you use the enclosing class, your favorite IDE will automatically suit that occurrence on a change of the class’ name (if you’re using vi or something similar to scourge yourself—at last your compiler tells you about it). Your logger will always be related to the current class’ name, a string will not give you that guarantee.

Anti-pattern

privat final static Logger LOGGER = LoggerFactory.getLogger("de.bar.Foo");

Pattern

private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

4. Logger-instantiation somewhere in between

A specialization of a common basic: define your class constants and attributes at the beginning of a class, not somewhere in between [1]. That significantly increases the readability and maintainability of your code. Apparently that also holds for loggers.

Anti-pattern

public class Foo {
	// ... code ...
	private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);
	// ... even more code ...
}

Pattern

public class Foo {

	private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

	// ... code
}

Reference

  1. [1] Martin, Robert C. 2008. Clean Code. Prentice Hall International

Apology in JavaDoc

Recently I stumbled about a lovely piece of JavaDoc:

/**
  * Because of possible changes of the Jar's name 
  * the actual name has to be determined programmatically.
  *
  * Apology to Baldr, Venus, Aphrodite, Lakshmi, Hathor 
  * and all remaining gods of beauty.
  */
public String getJarName() {
  .... // very ugly code
}
Top