Mark Pearce said:
Hi Niall,
So somebody, somewhere, *always* cares about the code's internal quality.
Of course, but my point was that code that has a nice design, but doesn't
work is much harder to sell to the customer than code that could have its
design improved, but it actually does the job. I'm not saying that I have a
"just get it done, we can make it pretty later" attitude, just that it's
more important to make sure the program does what is needed. From my
experience, once you get a decent sized system, you can be forever
"improving" the design, because the design of the architecture can never
really fully facilitate all cases of usage.
The developer in question thought that he *was* targeting a specific area of
code. He didn't know about the optimisation, and indeed had no way of
knowing about the optimisation without stepping through the code in a
source-level debugger.
Well, presumably the designer was aware of all the code in the method they
had written? The test should directly cause the methods being tested to be
run. To me, this is what testing in isolation means.
Of course in hindsight, it should have been coded better. But you've just
been arguing that the internal code quality doesn't really matter, as long
as the unit tests are satisfied. You can't have it both ways.
As had been said before, bad practice in coding, unit testing or step
through debugging can bring both sides down. If the coder had stepped
through the function without causing the other type of exception to be
thrown, then no one would know about it. All I'm saying is that better
coding of the original method would have allowed the unit test to pick up
the fault. Unit testing doesn't ensure perfect coding practices, and neither
does stepping through with a debugger...
planning to wrap the exception in a more meaningful one and re-throw. <<
There are various reasons to catch System.Exception, including the one you
mentioned. Another reason is to reverse a transaction after any exception.
Yet another reason is that many developers often put in a System.Exception
catch during testing, and forget to remove it. And yet another reason is
that developers sometimes don't realise that you shouldn't catch
System.Exception without rethrowing it. Finally, a few of the .NET base
class methods suppress all exceptions for "security" reasons, and just fail
silently.
Whatever the reason, you won't find this type of requirements/coding bug
unless you step through the code and find that an unexpected exception type
was being silently thrown and caught.
Any exception that escapes the function will cause the unit test to fail, so
if the coder was wrapping the exception and rethrowing, the unit test would
have caught it. If it was rolling back the transaction and then rethrowing,
the unit test would have caught that too. The only case that breaks the unit
test is when the exception is swallowed, which is bad practice. You can use
code smell type software to pick out this kind of thing. If you rely only on
the step through, you rely on the problem situation raising its head during
that one case of the step through.
Niall
