Tuesday July 24, 2007
Code Advice #15: Name getters properly! (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...
Comments are closed for this entry.
Posted by Sven Reimers on July 24, 2007 at 01:26 PM PDT #
Posted by Tor Norbye on July 24, 2007 at 04:26 PM PDT #
Posted by Sven Reimers on July 24, 2007 at 11:15 PM PDT #
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.
Posted by Ingo on July 25, 2007 at 12:29 AM PDT #
Yes, this doesn't happen for most people when writing "plain" getters and setters (and yes, NetBeans has the same functionality).
I think this is more likely to crop up when people are creating Builder objects (which the ProcessBuilder is an example of). These often don't utilize the standard bean pattern. For example, it's common to return "this" instead of void such that you can chain the setter calls:
pb.directory("/tmp").environment(mymap).redirectErrorStream(true).foo(x).bar(y);When you're writing a builder using this format having the "setter" name pattern isn't natural. Where they got into trouble is where the GET methods weren't properly named. For most of the methods there's no ambiguity; they got into trouble when the name suggested that it's performing an action. I think it's best to stick with clear getter names on builder objects even though the setters can be more brief and just have the "property" name.Posted by Tor Norbye on July 25, 2007 at 09:22 AM PDT #
Posted by Ben Loud on July 26, 2007 at 08:43 PM PDT #