Joel Spolsky is wrong. Returning status via error/return codes means that you can't trust the results of any method invocation — the returned value must be tested and dealt with.
This means that every method invocation returning such a value introduces at least one choice point (or more, depending on the domain of the returned value), and thus more possible execution paths through the code. All of which must be tested.
This code, then, assuming that the contract of Method1() and Method2() is to either succeed or throw an exception, has 1 possibly flow through it.:
foo.Method(...) ;
bar.Method(...) ;
If these methods indicate status via a return code, it all of a sudden gets very messy very quickly. Just returning a binary value:
bool fooSuccess = foo.Method(...);
if ( fooSuccess )
{
bool barSuccess = bar.Method(...);
if ( barSuccess )
{
// The normal course of events -- do the usual thing
}
else
{
// deal with bar.Method() failure
}
}
else // foo.Method() failed
{
// deal with foo.Method() failure
}
Returning status codes rather than throwing exceptions
- complicates testing
- complicates comprehension of the code
- will almost certainly introduce bugs because developers aren't going to capture and test every possible case (after all, how often have you actually seen an I/O error occur?).
The caller should be checking to make sure things are hunky-dory prior to invocation the method (e.g., Check for file existence. If the file doesn't exist, don't try to open it).
Enforce the contracts of your method:
- Preconditions. Caller warrants that these are true prior to method invocation.
- Postconditions. Callee warrants that these are true following method invocation.
- Invariant conditions. Callee warrants that these are always true.
Throw an exception if the contract is violated.
Then throw an exception if anything unexpected happens.
You code should be a strict disciplinarian.
