The “constructor that doesn’t construct” blooper of the day
Two people sent me this code blooper the other day, again, class and parameter names have been changed to protect the identity of the offender(s).
public SomeObject(
boolean lowSetting,
boolean highSetting,
boolean currentSetting,
Color lowColor,
Color highColor,
Color currentColor,
Color otherColor)
{
super();
lowSetting = this.lowSetting;
highSetting = this.highSetting;
currentSetting= this.currentSetting;
lowColor = this.lowColor;
highColor = this.highColor;
currentColor = this.currentColor;
otherColor = this.otherColor;
}
I have to admit I didn’t get it in the first couple of second, but it should be clear that this constructor is basically not doing anything. It basically overrides the constructor parameters with the current values in the object, which is more than likely not what we want to do.
BTW it also has a pet peeve of mine, it calls “super()” (the parent’s public no arg constructor). A lot of people don’t know that this is done automatically for you, no need to write code to for it.
Anyways, this one could have been prevented if you had a policy saying that method parameters should be named differently from class members. However, I personally like naming them the same. I guess it shouldn’t be unreasonable for the compiler to give you a warning when you do this, I’m going to guess most of the time you don’t want to do this.
Filed under: Code Bloopers on February 15th, 2008
I like to use final fields whenever possible, in which case this wouldn’t have compiled.
Using finals sounds like a good idea. You could make an argument that that should be a default too.
I’ve only done it when the parameters are used inside of an inner class in the method, which works, but doesn’t look very clean (some params are marked final others are not, and intent might not be clear).
I described one example with final fields in my blog:
http://asolntsev.blogspot.com/2008/02/null-initialization-in-java.html