Comments moved from
ConvertExceptions.
--
RussellGold
A related idea is to convert exceptions before they happen. For example, in a stack class, replace:
Object pop() {
try {
Object result = array[top];
top--;
return result;
}
catch (ArrayIndexOutOfBoundsException e) {
throw new NoSuchElementException();
}
}
with:
Object pop() {
if (top <= 0)
throw new NoSuchElementException();
top--;
return array[top+1];
}
The second version is arguably clearer because it doesn't need to guess what went wrong retrospectively. (And also because it doesn't have to undo the increment on 'top'.)
--
DaveHarris
I absolutely agree - in fact, I would consider the former to be poor programming style, since the code *can* determine that performing an operation would throw an exception, it should do so. This seems to me to be an example of
AssertionBasedProgramming described by
BertrandMeyer, except that Meyer didn't seem to care (at least in his original
ObjectOrientedSoftwareConstruction) about what exception was being thrown.
--
RussellGold
Of course, the problem occurs when you have a task in which evaluating the preconditions is as computationally expensive as doing the task. Parsing, dictionary lookup, and database operations are good examples of this. In such cases, the only efficient option often is to simply accept the cost of the exception unless the class designer has intelligently included an intermediate returning object that you can check for success before using the results.
The problem is that many users use this as a
HolyWar - on the one hand, throwing exceptions is inefficient (they're disturbingly slow in most languages), encumbers the debugger (which can normally consider throwing any exception as a real problem), and semantically confusing - better to
AvoidExceptionsWheneverPossible. However, this does not validate the equally crazy opinion of those who never ever want to throw exceptions - returning null or a magic number or somesuch nonsense is likewise silly, as it can result in the propagation of bad data (like the idea of returning a valid object on an empty stack as suggested below).
The solution, is of course, elsewhere. Any operation that can fail where it can be checked easily beforehand should simply allow the user to test if it will fail. In the case of a stack, Pop is coupled with
CanPop - it is up to the class user to determine what to do when it can't pop - the stack class itself just fails on Pop with an empty stack. If you want a stack that can pop when empty, just subclass it yourself and make your own
BottomlessStack.
In the case where checking is expensive, there are two popular OO exceptionless approaches:
- return an intermediate object that can be checked - eg Dictionary.Get(Key) returns a GetResult object with fields Success and Result, so that the caller can check for Success before consuming Result (which may be invalid).
- return multiple objects - in C-like languages this is generally done by returning the success value and having an "out" parameter set to the result.
In any case, the library should also provide exception-throwing forms as well, since often the failure cannot be handled (or expected) at the callsite and then the coder will have to tediously wrap the bad result in an exception object.
In short, every method that can throw an exception should be providing two approaches - one that allows the user to handle the problem at the callsite (without the exception ever being thrown - no catch blocks), and one that allows exception to fly.
Mmm, but I would disagree. I prefer the first version. This preference
is even stronger because at first I wouldn't even bother to convert
to exception (
YouArentGonnaNeedIt applies). So at first, I would do:
Object pop()
{
Object result = array[top];
top --;
return result;
}
Now if for some reason we care particularly about what exception it
is that gets thrown, we can always
ConvertExceptions, and thereby
get code example 1.
--
StephanHouben
Ouch. I really don't like that approach. It seems to me that it's very important to name your exceptions well... at LEAST as important as naming parameters and variables well, and nearly as important as naming methods and functions.
If everyone took the approach you suggest then until someone "care[s] particularly about what exception it is that gets thrown", the code will stay like this. But that will lead to a codebase
all of which throws A
rrayIndexOutOfBoundsException or N
ullPointerException for every conceivable situation (except the ones where "somebody cares"). This would be very confusing to work with, or to debug... you could tell that "something's wrong", but would have to trace the code to find out
what is is that was wrong.
See the discussion in
ConvertExceptions, which weighs heavily here. As that discussion might put it, the client of this code asked for a stack... the fact that it's implemented as an array not a linked list is really something the caller shouldn't have to be aware of.
Now, if your
YouArentGonnaNeedIt point is to avoid having to create the class N
oSuchElementException, then I sympathize -- you probably AREN'T going to need that. You could simply have boilerplate which, in a matter of 30 seconds, creates new exception classes, or you could use
throw new StandardException("NoSuchElementException");
instead. I don't think you should invest much work into CODING it, but I would strongly argue that you should invest work into NAMING things well.
--
MichaelChermside (but please, folks... contribute and/or refactor!)
I disagree with the example (which may not be a good example) - unless you expect the calling system to
VirtuallyNever call pop() on an empty stack, you should return a value that is usable in some fashion, since it really is no an
exception. Since you expect it, handle it. --
PeteHardie
I would indeed expect it to be rare. In fact, it would indicate a bug in the calling code. Suppressing information about bugs often does more harm than good. --
DaveHarris
It seems to me that the real problem here is the function Pop; Meyer uses this very function as a poster child for command-query separation.
--
CharlesYeomans
Is this identical in concept to a
GuardClause? If so, delete the page.
No... it's more a discussion of LookBeforeYouLeap vs CoupleLeapingWithLooking. In which case, it's still redundant, and should be merged with those pages.