Defensive Programming
January 27th, 2006 by Ivo
A few weeks ago, we had a major problem with software we'd written for a client. It was software for sending mailings to the client's customers. Suddenly there were many reports of clients receiving multiple mailings instead of just one.
The problem appeared to be in our test code. The software had a 'test' mode for testing the mailing by sending it only to the author and a small test team. It appeared that for some reason, all test mails were being mailed to the customers as well.
This problem would not have appeared if we had applied what I would like to call 'defensive programming'. Take a look at this example, which is an oversimplified version of how the software worked:
.... if ($mode=="test"} { $recipients = "testers@somedomain.ext"; } else { $recipients = "customers@somedomain.ext"; }
What happened was that due to some change in the software, this particular piece of code, along with a bunch of other code, was refactored into some other function, where the $mode variable ran out of scope. Result, $mode=="test" is never true, and we send all the test mailings to all our customers.
The defensive programming approach would be like this:
.... if ($mode=="production"} { $recipients = "customers@somedomain.ext"; } else { $recipients = "testers@somedomain.ext"; }
Would the same problem appear, the bug would have been less awkward, as only the testers would receive the mailing. Ofcourse this doesn't solve the actual bug, but it helps fight bad results.
In essence, you have to expect the worse. Write your code with Murphy in mind. Anything that can go wrong, will go wrong at some point. Especially when relying on (global) variables that are defined at some distance from the code where they are used, so it's easy to not notice the problem until it's too late.
P.S. We've released ATK 5.4 this week and, [shameless plug], my little .com adventure epointment.com has gone live this week in a very basic first version [/shameless plug]. For those interested: the epointment site is entirely written in ATK, while the backend server was written using Ruby on Rails, linked together via SOAP. What better way to compare ATK with Ruby on Rails
January 27, 2006 at 12:43 pm, Jules said:
If “$mode variable ran out of scope” then there is big chance that $mode is not declared and [code]if ($mode!="test"}[/code] risk to be true again, no ?
Personnaly, i will do :
[code]
$recipients = "testers@somedomain.ext";
if ('production_mode' == $mode) {
$recipients = "customers@somedomain.ext";
}
[/code]
January 27, 2006 at 12:53 pm, Ivo Jansch said:
You are right
My code example was incorrect and has the same problem; I’ve adjusted it to $mode==”production”
January 27, 2006 at 6:27 pm, Must@p Zone said:
Ivo Jansch at the “Achievo blog” has wrote a post about what he calls “Defensive programming”. The problem is very simple but not all developers that take care of such thing:
January 28, 2006 at 12:59 pm, Miguel said:
The Smarty errors aren’t very smart
http://www.epointment.com/epointment/eventplanner&lng=de
January 28, 2006 at 1:18 pm, Ivo Jansch said:
Thanks, fixed
(‘de’ is an unsupported language currently, it should’ve used fallback to ‘en’)
February 15, 2006 at 2:31 pm, Ants Aasma said:
Actually I’d say that the correct way to do defensive programming would be not to use else clauses unless it’s a true else, meaning that “in any other case, do this”. So the example would be:
[code]
if ($mode == "test") {
$recipients = "testers@somedomain.ext";
} elseif ($mode == "production") {
$recipients = "customers@somedomain.ext";
} else {
trigger_error("Invalid deployment mode '$mode'", E_USER_ERROR);
// depending on the level of robustness vs. correctness needed
// you can substitute E_USER_ERROR for E_USER_WARNING
// and give a safe default value (e.g. testers) for $recipient here
}
[/code]
This example actually lends itself well to the use of switch. This highlights the decision of default even better:
[code]
switch ($mode) {
case "test":
$recipients = "testers@somedomain.ext";
break;
default:
$recipients = "customers@somedomain.ext";
break;
}
// vs.
switch ($mode) {
case "production":
$recipients = "customers@somedomain.ext";
break;
case "test":
$recipients = "testers@somedomain.ext";
break;
default:
trigger_error("Invalid deployment mode '$mode'", E_USER_ERROR);
break;
}
[/code]