Checking return values in Java « Trifork Blog / Trifork: Enterprise Java, Open Source, software solutions

Checking return values in Java « Trifork Blog / Trifork: Enterprise Java, Open Source, software solutions

The other day I made a stupid coding error. It took me the better part of an hour to track it down. We all have those moments. Blame it on a bad night’s sleep, a brain fart, or perhaps the fact that your colleague at the next desk has been whistling that tune from Gotye’s “Somebody I used to know” (you know the one) all day, completely driving you crazy and taking your focus off your code.

Anyway. Here’s my coding error:

view source

print?

1	// If the finishedTest is an exam or a

2	// practiceTest, go to the next chapter or paragraph

3	getNextChapterParagraph(chapters, taskContentIdentifier);

4	if (taskContentIdentifier == null) return null;

5	return new Task(taskContentIdentifier, Type.ASSESSMENT);

And here’s the getNextChapterParagraph() method:

view source

print?

1

/**

2	* Returns the next paragraph or chapter given a list

3	* of chapters and the current chapter/paragraph.

4

*/

5	private ContentIdentifier getNextChapterParagraph(

6	List<Chapter> chapters,

7	ContentIdentifier contentIdentifier) {

8

9	// Do some stuff.....

10

11	return new ContentIdentifier(

12	chapter.getStream().getId(),

13	chapter.getContentId(),

14	paragraph.getContentId());

15

}

I’ve omitted and simplified some code to stick to the point, but I’m sure some of you have figured out what I did wrong by now. The getNextChapterParagraph() method doesn’t modify the ContentIdentifier instance passed as its second parameter; it returns a new one. Yet I wrote the calling code as if it did change the state of my taskContentIdentifier. D’oh!

A more common case of this is calling methods on immutable objects, like String. For example:

view source

print?

1	String string = "this is a test";

2	string.substring(10);   // Bad!

Now of course you can catch errors like this in your unit tests, which is how I found the error in the first place. But wouldn’t it be nice if there was some way to find out right when you make this mistake?

What’s wrong?

So what exactly is wrong with the code above? Simply put: the getNextChapterParagraph (and String.substring) method’s sole purpose is to return a value; it doesn’t change the objects whose references were passed to it as parameters, and it doesn’t change the state of the object it’s a method of. In other words, it has no side effects. Because of this, if we call the method, we should always do something with the return value. If not, why did we call it in the first place? (There is actually one reason you might, but I’ll discuss this later on.)

So we’d like to have the compiler or some tool warn us when we call a method that has no side effects, yet discard its return value. Any solution would have two parts:

1. Mark a method as having no side effects (perhaps it would be possible to determine by static analysis if a method has no side effects, but that’s a different blog post)
2. Warn when a method with no side effects is called and the return value is discarded (that is, not assigned to a variable, not used in an expression, and not passed to another method).

Enter: FindBugs and @CheckReturnValue

Read full article from Checking return values in Java « Trifork Blog / Trifork: Enterprise Java, Open Source, software solutions