Tuesday, July 24, 2007

Code Advice #15: Name getters properly!

WARNING: This blog entry was imported from my old blog on blogs.sun.com (which used different blogging software), so formatting and links may not be correct.


(See intro for a background and caveats on these coding advice blog entries.)




It's been a while since I've posted Java-coding opinions. But I couldn't resist on this one. The tip itself is straightforward:


A getter method's name should not sound like an action method.

Getter methods and setter methods should not have the same name.



I'm speaking from bitter experience since I just fixed a bug where my code had accidentally been calling a getter method, when I thought I was calling a setter method.
The method in question is java.lang.ProcessBuilder#redirectErrorStream. If you're launching a process, and you want to read its output, you may want to redirect its error output to its standard output such that you only have to read the input from a single source (assuming you don't care to distinguish between output and error.)



Well, my code was initializing the ProcessBuilder:


ProcessBuilder pb = new ProcessBuilder(args);
pb.directory(pwd);
pb.redirectErrorStream();



The problem here is that redirectErrorStream() does NOT redirect the error stream! It just returns false to tell me that it's not yet planning to redirect the error stream. The correct way to do this is


ProcessBuilder pb = new ProcessBuilder(args);
pb.directory(pwd);
pb.redirectErrorStream(true);


That's right - the setter and the getter are overloaded! Ewwwww!!!



This is the kind of thing a bug detection tool like findbugs could detect. It already warns if it sees you doing something similar, like calling String.trim() without storing the return value. Unfortunately, it didn't warn me about this case - so I filed a request. But this was a great reminder to run findbugs on my code again, which turned up some other interesting things to look at...


5 comments:

  1. I couldn't resist asking ;-) -
    Do you use SQE-Plugin (sqe.dev.java.net) for NetBeans 6? Or are you still running FindBugs with FindBugsGUI? (I assume your development environment is NetBeans6 daily)

    ReplyDelete
  2. Hi Sven, yes I did! I found the NB6 update center URL on the SQE home page - and I used it to install and run both Findbugs and PMD. Unfortunately, I ran into a bug where every time I click a source file or result I get an exception popup (something related to the code which messes with menus, presumably to plug into the various IDE context menus). I remember Sandip's code template module had the same bug; I should ping him to ask him how he fixed it. So, I had to disable the plugin. I figured it was just the build I was using. If you're not aware of this issue let me know and I'll be happy to provide more details or try any more recent builds you may have. It's a very nice plugin so I'd be very pleased to get a copy I can run daily!

    ReplyDelete
  3. Nice to here. I will try to get a working version back on-line ASAP.

    ReplyDelete
  4. I'm not sure when I handwritten the last getter (or setter) - create the properties and let the IDE do the rest:

    No typos, correct naming and far less work :-)

    I use Eclipse to do so (Source/Generate Getters and Setters) but am sure NetBeans will have the same functionality.

    ReplyDelete
  5. Right on Tor! I hope the NIO guys read this!

    ReplyDelete