In my code reviews, I often see engineers adding on to existing code even when a simpler solution is to modify it.
For example, take a look at this code:
void update_door_position(position_type position)
{
switch (position)
{
case OPEN:
set_door_opening(100); // fully open (100%)
break;
case SLIGHTLY_OPEN:
turn_on(30);
break;
case CLOSED:
set_door_opening(0); // fully closed (0%)
break;
default:
break;
}
}
The function allows us to request three different positions for a door, and it adjusts the door opening accordingly.
But sometimes the initial implementation does not have so many positions. Sometimes the first version of the code is simply this:
void update_door_position(bool open)
{
if (open)
{
set_door_opening(100); // fully open (100%)
}
else
{
set_door_opening(0); // fully closed (0%)
}
}
Here the door starts off with only an open position and a closed position, and then comes an additional request: “Sometimes our users would like to open the door just a little bit to let some air through, instead of opening it wide to walk through.”
This is where I see many engineers end up modifying the code into this:
void update_door_position(bool open, bool slightly)
{
if (open)
{
if (slightly)
{
set_door_opening(30);
}
else
{
set_door_opening(100); // fully open (100%)
}
}
else
{
set_door_opening(0); // fully closed (0%)
}
}
Even though this code does the exact same thing as the first chunk of code above, it is bulkier with more parameters to handle. Every time we want to call this function, we have to decide the value of two parameters instead of just one.
But what I find is, if the same engineers were asked to write the code from scratch for the three door positions, many would have written the first code above.
There seems to be a general reluctance to touch whatever has already been written (especially by someone else). Instead, flags are added here and there to achieve new functionality. However, this is not a good practice because it creates a code that is more difficult to read and to maintain. Imagine if more positions were added later for the door; how many flags would we use then?
When editing code, it is better to think about how the code should have been written if the new functionality had been there from the start. When we read code, we want to know what the code does right now, and not how many different phases had been used to add new functionality to the code. So we should write code that gives that kind of information, as simply as possible.
