Fix common library functions, or abandon then?
- by Ian Boyd
Imagine i have a function with a bug in it:
Boolean MakeLocation(String City, String State)
{
//Given "Springfield", "MO"
//return "Springfield, MO"
return City+", "+State;
}
So the call:
MakeLocation("Springfield", "MO");
would return "Springfield, MO"
Now there's a slight problem, what if the user called:
MakeLocation("Springfield, MO", "OH");
The called it wrong, obviously. But the function would return "Springfield, MO, OH".
The system was functioning like this for many years, until i noticed the function being used wrong, and i corrected it. And i also updated the original function to catch such an obvious mistake - in case it's happening elsewhere:
Boolean MakeLocation(String City, String State)
{
//Given "Springfield", "MO"
//return "Springfield, MO"
if (City.Contains, ",")
throw new EMakeLocationException("City name contains a comma. You probably didn't mean that");
return City+", "+State;
}
And testing showed the problem fixed. Except we missed an edge case, and the customer found it.
So now the moral dillema. Do you ever add new sanity checks, safety checks, assertions to exising code? Or do you call the old function abandoned, and have a new one:
Boolean MakeLocation(String City, String State)
{
//Given "Springfield", "MO"
//return "Springfield, MO"
return City+", "+State;
}
Boolean MakeLocation2(String City, String State)
{
//Given "Springfield", "MO"
//return "Springfield, MO"
if (City.Contains, ",")
throw new EMakeLocationException("City name contains a comma. You probably didn't mean that");
return City+", "+State;
}
The same can apply for anything:
Question FetchQuestion(Int id)
{
if (id == 0)
throw new EFetchQuestionException("No question ID specified");
...
}
Do you risk breaking existing code, at the expense of existing code being wrong?
