Posts Tagged ‘Refactoring’

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();

A quick refactoring using TestNG’s expectedExceptions

I recently crossed a TestNG-test which looked like this:

@Test
public void noResultsTest() {
	// test
	String query = " ";
	Exception exception = null;
	List foos = null;
	try {
		foos = barService.search(query);
	} catch (SystemError e) {
		// input was wrong
		exception = e;
	}

	// assert
	Assert.assertNotNull(exception, "There was an exception");
	Assert.assertNull(foos);
}

Obviously this test should check that barService#search throws an Exception in case the input is a blank.

But there are a few things to mention:

  • TestNG is capable of checking if an expected exception was thrown in a test. Simply use @Test(expectedExceptions={..}).
  • The check that Assert.assertNull(foos) holds true is useless. Either no exception was thrown and Assert.assertNotNull(exception, "There was an exception") will fail or an exception was thrown and foos will not be assigned with any value.

So the test could be rewritten like this:

@Test(expectedExceptions={ SystemError.class })
public void keineBerufeTest2() throws SystemError {
	foos = barService.search(" ");
}
Top