I was reminded once more today just how important it is to write minimal APIs that don’t expose more than they have to. Briefly I had code like this:
private boolean flag;
public boolean getFlag() {
return this.flag;
}
public boolean setFlag(boolean value);
this.flag = value;
}
Pretty boilerplate stuff, I think you’ll agree.
However I noticed that after some refactoring that merged a couple of classes I was now only calling getFoo() from within the same class (or at least I thought I was) so I marked it private. Eclipse promptly warned me that the method was unused so I deleted it. Then Eclipse warned me the field was unread. That seemed wrong so I looked closer and yep, it was a bug. The feature the flag was supposed to control was always on. During the refactoring I had failed to move the use of the flag field into the new class. I added a test to catch this, and fixed the problem.
What’s interesting about this example is that I found the bug only because I was aggressively minimizing the non-private parts of my API. The less public API a class has, the fewer places there are for bugs to hide. The less public API there is, the easier it is for analyzers–static, dynamic, and human–to detect problems.
Many programmers subscribe to a cult of extensibility: Extensibility is always good. Opening up the API makes it more testable. Compile time static type checking really doesn’t help anyway. You might need it someday so put it in now. Maybe you don’t need it, but someone else might.
This way lies madness. Most extensibility points are never used. How many getters and setters are actually invoked? Not as many as you’d expect, and even fewer if test classes are not considered. How many non-final classes in your code actually have subclasses? How many of these classes have actually been designed and documented for extensibility? How many non-final fields actually are mutated after construction? Certainly some are. Not all methods and fields can or should be final. Some classes are designed to be subclassed. Some fields do need getters or less frequently setters. But none of these features should be added to your classes out of habit.
For maximum safety, remove as much as you can and lock down what you can’t. Make classes (and/or methods) final by default. Make fields final and immutable. Don’t mark a method public if it can be package protected instead. Don’t routinely add getters and setters for each field; and if you do need a getter or setter, just add the one you need, not both. In this case, I needed a setter, but the getter was gratuitous and removing it revealed a real bug.
Follow the YAGNI principle: You Ain’t Gonna Need It. Never add an extension or access point–be it a method, a field, a non-final class, etc.–until you know you need it. This will improve your code’s robustness, thread safety, security, speed, and more.
Previous Topic
