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:

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)

Posted by Sven Reimers on July 24, 2007 at 06:26 AM PDT #

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!

Posted by Tor Norbye on July 24, 2007 at 09:26 AM PDT #

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

Posted by Sven Reimers on July 24, 2007 at 04:15 PM PDT #

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.

Posted by Ingo on July 24, 2007 at 05:29 PM 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 02:22 AM PDT #

Right on Tor! I hope the NIO guys read this!

Posted by Ben Loud on July 26, 2007 at 01:43 PM PDT #

Post a Comment:
Comments are closed for this entry.
About

Tor Norbye

Search

Archives
« April 2014
SunMonTueWedThuFriSat
  
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
   
       
Today