Sunday, June 9, 2013

Aggressive Coding

Why you should code agressively, not defensively

Defensive coding is a concept that has it's origin in the absolute first ideas about programming as a craft, at least 40 years ago. Wikipedia describes it as something "intended to ensure the continuing function of a piece of software in spite of unforeseeable usage of said software". Apparently principles that can be characterized as defensive coding techniques are still being taught, or at least not actively discouraged.

Defensive coding sounds good in theory, but in practice it tends to excarberate the problems in question, and clutter up the code making it harder to understand and refactor.

A typical idea in defensive coding, is the Wikipedia example of copying strings from one buffer to another. The idea is that if the caller provides a longer source string than expected, this might in the case of C/C++ open up for the classic buffer overrun security vulnerability.

The defensive coder will, as the example shows, provide a function that checks the maximum buffer length and silently refuses to copy more than that to the destination buffer. This is bad and dangerous!

The defensive coder has now just hidden a serious bug in the calling code. If the contract states that 1000 characters is the maximum length of the input, the caller must ensure this and the callee refuse to accept anything that violates the contract.

The agressive coder will instead throw an exception or simply terminate the program if the function is called with a source larger than the allowed 1000 characters. This is safe and secure programming!

In terms of my current preferred language C#, I see this principle violated in a variety of ways. One frequent pattern is checking return results from other library functions for NULL, or empty strings etc, and then attempt to silently do something despite that this was unexpected. This typically indicates that the programmer does not know the contract of the method s(he) is calling. Do know the contract and ensure to follow it when providing input, and assume that it is followed for the produced output!

I recently rewrote a major functionality for a client and this also involved updating and refactoring the dependent code. To my horror a huge amount of code was devoted to checking the outputs of other methods, even to the extent of catch-all clauses silently ignoring any and all problems.

Instead of checking outputs from other other code because 'maybe it can return a NULL' - find out if it can, and what the appropriate action is! If you can't find out with reasonable effort and it doesn't seem a useful response, just use the return value and let your code blow up with a NullReferenceException should it in fact happen. This will alert you to the problem, and you can then find out what you really should do about it. Do check the inputs to your own code, and when the caller violates your contract report this with an exception.

Controlled crashing is good when it's because a caller violates a contract!

Agressive coding increases the chance of problems being caught and fixed early, and reduces the amount of clutter in the code immensly. This in turn lets you concentrate on what your code should do, instead of what someone elses code should not.