public class Employee {
private boolean manager;
public boolean isManager() {
return manager;
}
...
}
...
if (employee.isManager()) {
calculateManagerPay();
else {
calculatePay();
}
...
The problem is that now there are potentially many other parts of the system that care about whether an employee is a manager or not. Some of those other parts will not be changed, and defects will be reported against the system when those parts of the application are used. That's the "if bug".
Encapsulation was broken when manager was exposed to the rest of the system. Object Oriented Design tells us to not expose the implementation details of a class to the rest of the system.
There are many better ways to implement this, and I'll save what is perhaps the simplest way, Object Flag, for next time.
Posts
5 comments:
The breaking of encapsulation you mentioned exists in all uses of setters/getters--not just booleans.
Setters and getters are a terrible pattern. They were used in Java to handle a very special case, "Java Beans", and that is virtually the only case they are good for. In fact, now that we have annotations they aren't even good for that.
For new OO programmers, getters and setters are often a first example (because they are easy) and they appeal to their procedural programming mentality, so they tend to be hard to get rid of.
One more thing. This particular case generalizes to:
switch typeof employee
case manager:
case other:
Any time you are doing a switch on the type of something, it's a bad OO code smell--It indicates that your design should have two classes providing different implementations of a single method (either through inheritance or better yet, an interface)
This is essentially the problem and solution you were pointing out, I just generalized it a little.
http://blogs.concedere.net:8080/blog/discipline/software+engineering/?permalink=Getters-and-Setters-are-A-OK.html
Best,
Laird
Nice entry.
I think your bug is commonly known as 'polymorphism over conditional logic'.
Looking at your mentioned 'if bug pattern' the main concern i have is that it will break the 'open closed principle' as soon as you'll add some other types of employees (because you may have to add another if case in order to handle the new type).
Greetings
Mario Gleichmann
Thanks. Mario, you are exactly right. I'm more than a little ashamed at missing the obvious OCP violation on top of the encapsulation problem.
Thanks for reading.
Post a Comment