Wednesday, April 02, 2008

Property Getters with Bad Manors

In the sources of a 3rd party library that I'm using I found the following code:
public static int Counter {
   get {
      int cnt = counter;
      counter = 0;
      return cnt;
   }
}
Ok, the documentation says that calling this getter will reset the counter. However... Still, in my opinion this is bad coding practice. Getters shouldn't modify the object. The reason for that is that some IDE's use the getters for displaying object data in debugging sessions. As a consequence debugging code and looking at the member of this object influences the outcome of the debugging session. I call this bad manors of that getter. The better approach would have been to use a method with a more intuitive name for example "ReadAndResetCounter()". That would a) avoid accidental modification during debugging sessions, b) follow the good coding practice of getters being non-modifying accessors, and c) would express the actual functionality of the code in a more understandable way. Some recommendations as a take-away:
  • Don't implement getters that modify the object
  • Instead use a method with a descriptive name

That way you will make it easier for other engineers to use your code and/or library.

(Now I'm wondering how long it will take until Charlie reads this. I'm sure he will agree with me!)

No comments:

Post a Comment

All comments, questions and other feedback is much appreciated. Thank you!