Property accessors can be evil!!

Yesterday I came across  this on the daily wtf.

Generally I admit I default to adding getters an setters, quite often just to offer sacrifice to the java bean standards, even though this is very often just noise and syntactic sugar.  But I’m always wary when I see logic in a getter/setter, the example in this article is a fairly harmless one (as in its unlikely to break the application).  I’d be quite reluctant to do a redirect in a getter, thats just evil. Imagine trying to find that bug. The basis of that is that getters, when it comes to Java in particular, are so frequently used by ‘infrastructure code’ that you’re not 100% in control of (which of course Reflection is to blame for but that’s another story) . This opens up for way too many wtf’s in your everyday life.

I discovered this getter in a struts form the other day;


public List getDomainObjectList(){
return callMethodThatDoesIOAndAppliesBusinessLogic();

}

The struts form getter is called from a jsp, but since we’re also using apache commons reflection util to populate/scrape values from the form, and this field masquerades as a proper Bean property,this code gets called 3 times per page load. No wonder the error log is spammed with errors when the domain objects aren’t behaving.

And a few days earlier I had I discovered this piece of code, checked in over 3 years ago which just clearly does not do what it says on the tin


public void setExpired(boolean expired){

   this.expired=expired;

}
public boolean isExpired(){

	return this.userDetails().getExpiryDate()== null;
}

Now this is just poor coding, I know and this is nothing code reviews will not cure, but personally, I’d quite happily  do without these kind of surprises, which makes  removing the getters/setters all together quite attractive. They should most certainly come with a license.

The point is, if you are putitng any non trivial logic in your gettters and setters, you need to think long and hard about where they are being used, and then reconsider your decision.

//J

Advertisements



    Leave a Reply

    Fill in your details below or click an icon to log in:

    WordPress.com Logo

    You are commenting using your WordPress.com account. Log Out / Change )

    Twitter picture

    You are commenting using your Twitter account. Log Out / Change )

    Facebook photo

    You are commenting using your Facebook account. Log Out / Change )

    Google+ photo

    You are commenting using your Google+ account. Log Out / Change )

    Connecting to %s



%d bloggers like this: