Mathematics is the art of giving the same name to different things.
— Henri Poincare
The other day I ran across the following in a pull request:
This got me thinking.
I personally would prefer to not use “get” because I feel that “get” has a lot of connotation to it. “Get” implies getter (and possibly setter) functionality. Getter methods in java are often boilerplate code, which causes people to make other assumptions. This reminded me of an issue that happened earlier in my career that should have been easy to find but wasn’t.
The standard boilerplate code for a getter and setter generally looks like this:
The cost defaults to zero if not set, otherwise the cost is whatever was set. Would you write a unit test for this? (Of course not, its boilerplate code.) I usually don’t even write the getters and setters, I just have the IDE generate them. You can even set the code coverage tools to ignore getters and setters when calculating out how much coverage there is.
Now someone had previously written the following code:
This code greatly reduced the number of
UsefulObject that had to be processed because while there were reasons for the objects to exist, only a small number of the
UsefulObject instances had a cost and were used at any point in time.
The cost field of a
UsefulObject now had an additional connotation. The application worked fine for several months but then one day the application stopped working. Initial investigations showed it had a significant increase in CPU usage. The change that caused this situation was well intentioned and was done an attempt to fix something else:
This change did not impact any of the existing unit tests as no one had written unit tests for the getters and setters (but then again who does?). Everyone was thinking of and talking about the objects in preconceived terms. They were looking at the recent code changes in the application but unfortunately
UsefulObject was declared and defined in a separate library. It wasn’t until preconceived ideas were thrown out that the performance issue was fixed.
Names and labels often have meanings beyond what they are directly stating. There was quite an argument over what the actual bug was. Some felt that using the value of the cost field to determine which objects to process was the cause. Their suggestion was to create a new field to directly indicate whether to process or not process the object. Others felt that changing cost to always return a value was incorrect. They wanted to add another method to either return cost or
0.01f if cost was less than or equal to zero.
The real cause of the problem was the connotation around the cost field. By explicitly stating how fields are used would have prevented any misunderstanding:
By adding a new method, the
UsefulObject class encapsulates how to determine if it will be processed. Adding additional methods has some advantages over adding a new field which will act as a flag. First, adding a method to a class does not take up additional memory, which could cause a problem in applications which are memory sensitive. Secondly, if an application serializes the object - if the objects are stored for later use or are logging - adding a flag would require additional code for versioning and backwards compatibility.Finally, it will be easier to change the code in the future if the business rules around which
UsefulObject instances to process should need to change.
Changing a getters expected behavior often causes bugs but not changing the getter often leads to code that repeats itself:
If the call to
getCost is in one place, then the above approach is acceptable. However, if it is in many places, then this breaks the DRY (Don’t Repeat Yourself) principle. I suggest leaving the getter alone and adding a second method -
getCostOrElse - to access the value or return a default value:
This approach is much more readable and easier to adjust than checking the value returned by the getter each time. Take this into consideration, what would happen if zero became a valid cost? When you are developing code, it is easy to change the code. But once the applications has been released into the wild, it often becomes a lot harder. The time and effort to code review and test the change becomes a function of the size of the change.
Strive for clarity in your objects and methods. Your objects and methods should have a well defined purpose. This improves the readability and maintainability of your code. You will be thankful the next time you have to go back and change the code.