Monday, October 26, 2009

Watch your booleans!

Here's a simple reminder that, whether you're coding or doing a code review (and you ARE doing code reviews, right?), double check your booleans.
private void EnableDisableFancySchmancyControl(bool enable)
{
_fancySchmancyControl.ReadOnly = enable;
}

We have a piece of code like this in the software I work on daily. This tiny little piece of code, so minuscule, so obviously correct, caused an emergency promotion the day its bug was found because it didn't allow some users to do their work properly.

Why, you ask, do we have such a simple method? Wouldn't it do to just call
_fancySchmancyControl.ReadOnly = enable;
in the calling method? Yes, normally it would. The objective of this piece of code is to normalize the action with the question "Should I allow the user to edit the information in this control?", so that you don't end up with things like:
private void EnableEditing()
{
_control1.Enabled = true;
_control2.Readonly = false;
_control3.Enabled = true;
}

but instead have:
private void EnableEditing()
{
EnableDisableControl1(true);
EnableDisableControl2(true);
EnableDisableControl3(true);
}

Now, the error from our original snippet should be obvious when you're looking for it: the line should be
_fancySchmancyControl.ReadOnly = !enable;
That is, there should be a not (!) in front of enable; a control that is read-only is not editable by the user (which, remember, is our objective).

All of that to say: be extra careful with your booleans; even the simplest code can fall prey to this bug!
Post a Comment