{"id":8389,"date":"2013-07-02T14:30:41","date_gmt":"2013-07-02T12:30:41","guid":{"rendered":"https:\/\/blog.trifork.com\/?p=8389"},"modified":"2013-07-02T14:30:41","modified_gmt":"2013-07-02T12:30:41","slug":"checking-return-values-in-java","status":"publish","type":"post","link":"https:\/\/trifork.nl\/blog\/checking-return-values-in-java\/","title":{"rendered":"Checking return values in Java"},"content":{"rendered":"
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. \n<\/p>\n
Anyway. Here’s my coding error:<\/p>\n
<br>\n\/\/ If the finishedTest is an exam or a<br>\n\/\/ practiceTest, go to the next chapter or paragraph<br>\ngetNextChapterParagraph(chapters, taskContentIdentifier);<br>\nif (taskContentIdentifier == null) return null;<br>\nreturn new Task(taskContentIdentifier, Type.ASSESSMENT);<br>\n<\/pre><\/p>\n
And here’s the getNextChapterParagraph() method:<\/p>\n
<br>\n\/**<br>\n * Returns the next paragraph or chapter given a list<br>\n * of chapters and the current chapter\/paragraph.<br>\n *\/<br>\nprivate ContentIdentifier getNextChapterParagraph(<br>\n List<Chapter> chapters,<br>\n ContentIdentifier contentIdentifier) {<\/p>\n<p> \/\/ Do some stuff.....<\/p>\n<p> return new ContentIdentifier(<br>\n chapter.getStream().getId(),<br>\n chapter.getContentId(),<br>\n paragraph.getContentId());<br>\n}<br>\n<\/pre><\/p>\n
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()<\/code> method doesn’t modify the ContentIdentifier<\/code> 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<\/code>.<\/strong> D’oh!<\/p>\n
A more common case of this is calling methods on immutable objects, like String<\/code>. For example:<\/p>\n
<br>\nString string = "this is a test";<br>\nstring.substring(10);\t\/\/ Bad!<br>\n<\/pre><\/p>\n
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?<\/p>\n
What’s wrong?<\/h2>\n
So what exactly is wrong with the code above?<\/strong> Simply put: the getNextChapterParagraph<\/code> (and String.substring<\/code>) 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<\/em>. Because of this, if we call the method, we should always do something<\/em> 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.)<\/p>\n
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:<\/p>\n
\n
Mark a method as having no side effects<\/strong> (perhaps it would be possible to determine by static analysis if a method has no side effects, but that’s a different blog post)<\/li>\n
Warn when a method with no side effects is called and the return value is discarded<\/strong> (that is, not assigned to a variable, not used in an expression, and not passed to another method).<\/li>\n<\/ol>\n
Enter: FindBugs and @CheckReturnValue<\/code><\/h2>\n
As it turns out, FindBugs, the code analysis tool most of you are probably familiar with, has exactly the tools for this. I’ll describe how to set-up FindBugs in Eclipse further down. First, the solution to our problem: the @CheckReturnValue<\/code> annotation. Simply put, @CheckReturnValue<\/code> marks a method as having no side effects. If FindBugs then detects a method call to such a method where the return value is discarded, it emits a warning.<\/p>\n
To use @CheckReturnValue<\/code> you’ll need the findbugs-annotations.jar<\/code> on your classpath. Maven users can add the following dependency to their POM:<\/p>\n
Now, let’s try to call this method without using the return value:<\/p>\n
<br>\npublic class CheckReturnValueExample {<\/p>\n<p>\tpublic void testStringHelper() {<br>\n\t\tStringHelper stringHelper = new StringHelper();<br>\n\t\tString str = "string";<br>\n\t\tstringHelper.shift(str);<br>\n\t\t\/\/str = stringHelper.shift(str);<br>\n\t\tSystem.out.println(str);<br>\n\t}<br>\n}<br>\n<\/pre><\/p>\n
Running FindBugs yields the following warning:<\/p>\n
Bug<\/strong>: return value of StringHelper.shift(String) ignored in nl.trifork.examples.checkreturnvalue.CheckReturnValueExample.testStringHelper()<\/p><\/blockquote>\n
Now, we can’t annotate methods in JDK classes with @CheckReturnValue<\/code>, but luckily FindBugs already seems to have checks for most common cases built-in:<\/p>\n
<br>\npublic class CheckReturnValueExample {<\/p>\n<p>\tpublic void jdkStringExample() {<br>\n\t\tString str = "this is a test";<br>\n\t\tstr.substring(10);<br>\n\t\tSystem.out.println(str);<br>\n\t}<br>\n}<br>\n<\/pre><\/p>\n
FindBugs:<\/p>\n
Bug<\/strong>: return value of String.substring(int) ignored in nl.trifork.examples.checkreturnvalue.CheckReturnValueExample.jdkStringExample()<\/p><\/blockquote>\n
Exceptions<\/h2>\n
So are there any cases in which it is valid to discard the return value of a call to a side effect-less method? Actually, there is, but it stems from the fact that none except the most trivial of methods are truly without side effects. Consider String.substring()<\/code>:<\/p>\n
![\"Installing](\"https:\/\/trifork.nl\/articles\/wp-content\/uploads\/sites\/3\/2013\/06\/Install-FindBugs-STS1.png\")
![\"Enabling](\"https:\/\/trifork.nl\/articles\/wp-content\/uploads\/sites\/3\/2013\/06\/FindBugs-Run-automatically1.png\")
![\"A](\"https:\/\/trifork.nl\/articles\/wp-content\/uploads\/sites\/3\/2013\/06\/FindBugs-warning.png\")
<\/a>![\"The](\"https:\/\/trifork.nl\/articles\/wp-content\/uploads\/sites\/3\/2013\/06\/Bug-Info-panel1.png\")
![\"\"](\"https:\/\/trifork.nl\/articles\/wp-content\/uploads\/sites\/3\/2022\/02\/Blog-Banner-1-1024x256.png\")
<\/a><\/figure>\n","protected":false},"excerpt":{"rendered":"