Trap Bad Parameters

Problem

If some one calls your method with bad parameters, it won't work right. To make it break cleanly, TrapBadParameters:

  public void process (int index) {
if (index > maxIndex() )
return
// do something
  }

Why?

Easier debugging
Keep bugs from damaging important data or processes

What to do with bad parameters

Throw an exception
Return (ignore them)
Try to correct them

DesignByContract

If you are doing DesignByContract such that your asserts are not present in the deployed code, you might TrapBadParameters in addition to your asserts. Whether or not you do so will depend on the cost of an uncaught bad parameter. TrapBadParameters and DesignByContract together look like this:

  public void process (int index) {
assert( index <= maxIndex() );
if (index > maxIndex()) {
// handle error.
return;
}
// do something
  }

When the assert goes away, the trap remains.

Alternatively

  public void process (int index) {
if (index > maxIndex()) {
assert(false);
// handle error.
return;
}
// do something
  }

I don't see the point of this. Why duplicate the assert? If you really want to have the check performed at run-time, why not just leave the assert enabled? OnceAndOnlyOnce, you know?

While this may seem like a good idea on the surface, it really isn't because it creates complex bloated code. Internally to your codebase, you can assume all the code is perfect because you wrote it and you can fix it. Assume the caller knows what it is doing. If the program crashes, fix the caller, not the callee. Assertions help detect broken callers without bloating the release code, so they are a nice inbetween compromise. On the other hand, when your system touches the outside world (user interface, network, etc.), then you have to be rigourous. Fundamental principle: No code has no bugs!

This bloated code can actually be a feature. ExtremeProgrammingChallengeNineteen mentions an example of the "system touches the outside world" clause. I quote:

What the (real) XYZCorp actually do is to write lots of code that they, somewhat strangely, call 'belts and braces' code (see BeltAndBraces). This is code that continually checks the reasonableness of various aspects of the data structures that pass back and forth to the driver, and pops ups and says ´please call support and say...' whenever anything unexpected appears. None of this code ever gets tested, and most in fact has never been executed, but the bits that have have saved a lot of time and embarrassment.

But, this bloated code can also be a feature for the "internal to your codebase" situation. In my current employment, the group I'm in needs to respond quickly to problems that crop up in the field. I'm using specific error messages that report when the first thing goes wrong. This apparently makes the bugs much easier and faster to fix. In fact, our system has evolved to the point that just about every function returns an integer error code to indicate success vs. specific failure.

Software evolves over time. You can't remove the scaffolding and hope that nothing will ever change again.

Trapping bad parameters is important because that's where the error message can be most semantically meaningful. Shipping code that does ridiculous, dangerous, awful things that no one would want, and then blaming the user for not reading all the caveats and provisos in the documentation is a hostile approach. Error messages are part of the API, just like documentation and the binaries/source/headers/etc that the code actually links to.

There are certainly cases in which performance, code size, etc. are more important than developer effort. Those are exceptional cases compared to the general trend in which computers' time is getting cheap and developers' time remains extremely expensive.

As for "complex bloated code", that's just nonsense. There's nothing bloated or confusing about something like this:

function divide(numerator, denominator) {

   if (0 == denominator) { complain("You can't divide by zero"); }
   ...

}

It's obvious what it does because of the condition and error message, and it compiles down to approximately one CPU instruction to do the comparison, which takes a few nanoseconds to execute.

Compare that to a gigantic stacktrace with a null pointer exception or a bus error or something like it - no help for the developer whatsoever, and the caller's typical remedy is an all night debugging saga with an elaborate debugging rig to try and figure out what the hell went wrong. The user shouldn't have to step-trace inside your code and down into another library or kernel to figure out what your code asked it to do. In creating a library, you're defining a black box and documenting what it does. Allowing all sorts of bizarre undocumented functionality to reside in your black box too, and leaving it up to the caller to figure out what on earth is going on, is just bad.

Also - if your code shouldn't be expected to check arguments or do the right thing, why should the caller, or the code you call? Where does the buck stop regarding the responsibility for writing code that works? By not checking arguments, you're essentially requiring the whole outside world to work properly, or else your code breaks in unpredictable, undocumented ways. That may make your job easier because you can point the finger, but it's an unrealistic expectation, and unless your goal is to have a non-working system that is non-working due to somebody else's bugs, you should actually be less trusting of the other code that's involved in your system.

CategoryDefensiveProgramming