Design - Run into an exception or avoid it?

Question

I have got a short design question to all of you. I have got a method which must be executed as fast as possible, but it also must give information about occurred errors by an exception.

It may happen that the function is called a thousand times within a loop, but exceptions will occurr very rarely (<1% of the values will cause an error). The function only computes very simple mathmatically operations and do not call any methods, using LINQ or something else.

Ok that's the situation so far, and the following describes the two scenario I have got to solve this method.

My own solution was the most common way, just check each parameter before the calculation (to prevent exceptions in the ramaining method):

int FastMethod(int number)
{
    if (number <= 0)
    {
        throw new ArgumentOutOfRangeException();
    }
    // (...) more parameter validations

    // do some operations with the number here
}

The other solution which was recommended to me was, running into the error and only re-throw the catched exception:

int FastMethod(int number)
{
    try
    {
        // do some operations with the number here
    }
    catch (Exception ex)
    {
        throw (ex);
    }
}

So what would you recommend?

The second scenario doesn't need to call all these if statements that may raise the performance, but it looks poor designed in my opinion. May you can teach me :)

`catch (Exception ex) { throw ex; }` is completely useless code. That's what the runtime does anyway. If the `catch` block does nothing other than rethrow, it shouldn't be there. — michaelb958--GoFundMonica, Apr 19 '13 at 12:31
I am Not sure how many parameter validations are involved. In order to retain accuracy of validations you can group the validations appropriately with `||`s and `&&`s. Other way would be to pass a number which is well validated before hand to avoid FastMethod validating it. — Vishy, Apr 19 '13 at 12:38
@TimKrüger I'm not recommending anything; I'm just pointing out some dead ASCII weight. — michaelb958--GoFundMonica, Apr 19 '13 at 12:39
Thanks Vishy, I am currently reading some posts about parameter validation :) In my case the number is passed by code and I have to check it. But like michaelb958 said, the second scenario is complete useless, even if I want to add a custom description to the exception it would be quite useless because i document the same exception twice in this case... — Tim Krüger, Apr 19 '13 at 12:41
Neither code sample avoids the exception. (On Java (6)) I found throwing/catching exceptions to be massively slow compared to just a simple number comparison for validation (exception on 1% or less of cases, like in the region of 10-100x slower). Returning a identifiable value to indicate a problem should be faster, though some will argue it's worse design. — Bernhard Barker, Apr 19 '13 at 12:43
Here is an interesting post on speed of exceptions http://stackoverflow.com/questions/299068/how-slow-are-java-exceptions — Bart, Apr 19 '13 at 12:52
@michaelb958 It's worse then useless, you lose the original stack trace. — asawyer, Apr 19 '13 at 13:01
@asawyer I wasn't sure about that when I originally pointed it out; I elected not to mention it. — michaelb958--GoFundMonica, Apr 19 '13 at 13:03
@TimKrüger This article might be useful for you: http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx — asawyer, Apr 19 '13 at 13:06
@asawyer wow that post changed my mind AND it's exactly what i was searching for :D Thanks — Tim Krüger, Apr 19 '13 at 13:13
@TimKrüger My advice - Read every word Eric Lippert has ever written. — asawyer, Apr 19 '13 at 13:15

Pressacco · Accepted Answer · 2013-04-19T14:47:17.270

1

If I understand your question correctly, you are asking: should I (1) allow an exception to occur, or (2) should I be proactive and test the input variables (think: defensive driving)?

My Thoughts

If at all possible, you should try to detect erroneous data as soon as possible. The first method in the call chain should deal with the problem.
If the FastMethod method is outward facing (e.g. a method on a public API), then I would definitely test the input parameters (i.e. number)
[bullet not relevant]
Don't get caught by the trap of optimizing too early. Is performance really an issue?
As michaelb958 pointed out, your second example doesn't do anything. Worse yet, your are needlessly adding additional clock cycles by simply catching and re-throwing the exception.

Without having more information, I would say: - use scenario 1: test the input parameter to ensure that it is in range

Additional Reading

UPDATE 1 As MSalters has correctly pointed out in his LibFoo::WiggleBar() comment: the parameter check should be placed at the lowest level in the call chain. Generally speaking I would implement my code this way. Thank you MSalters for setting me straight.

My initial thoughts re: bullet #1 were: if performance was indeed an issue, one way to gain back clock cycles is to avoid unnecessarily adding to the call stack.

@Tim Krüger: Unless you are 100% certain that performance is going to be an issue... I would focus on writing maintainable + easy to read + bug free code.

UPDATE 2 Removed bullet 3.

edited Apr 19 '13 at 14:47

answered Apr 19 '13 at 13:16

Pressacco

2,815
2
26
45

Disagree. If there's already existing code to detect an error (in particular, if that code is in a well-tested library such as LINQ), then duplicating the checks is generally bad. Additionally, if your check is too strict, you may cause correct input to be rejected. If it's too weak, you may still get errors from the original check. – MSalters Apr 19 '13 at 13:24
@MSalters: I think we are on the same page. "If there's already existing code to detect an error then duplicating the checks is generally bad". I completely agree. That was my point "If at all possible, you should try to detect erroneous data as soon as possible. The first method in the call chain should deal with the problem.". For example: if *FastMethod* is part of a public API, then it is probably the first place the *number* parameter can be tested. Ideally, the parameter check should only be performed once. Of course the answer can vary depending on external factors. – Pressacco Apr 19 '13 at 13:37
if I understood OP correctly, he wanted to say that he is NOT using linq in the performance critical method? `and do not call any methods, using LINQ or something else` not sure though... – Dennis Apr 19 '13 at 13:51
still, i totally agree with your answer, so +1 – Dennis Apr 19 '13 at 13:52
I'm not so sure. If `FastMethod` internally calls `LibFoo::WiggleBar()`, and LibFoo has the correct `number` check in place, then you should not attempt to duplicate that check inside `FastMethod`. There's nothing to be gained, and a lot that can go wrong. From the outside, `FastMethod` callers won't know who generated the exception anyway. – MSalters Apr 19 '13 at 13:53
@MSalters: Given your example, it is entirely possible that more than one method could call `LibFoo::WiggleBar()`. If this is the case, then I 100% agree with you: the code check should be place at the lowest level (*i.e.* `LibFoo::WiggleBar()`). – Pressacco Apr 19 '13 at 14:07
@Chips_100 : You are right re: LINQ. I misread the OP. Thanks! – Pressacco Apr 19 '13 at 14:35

score 0 · Answer 2 · answered Apr 19 '13 at 15:48

Another option would be to move the catch block above one level to the caller, so the try/catch block is not setup again for each iteration. Works of course only if you don't want to continue the loop after an exception.

General performance advice: move everything out of a loop that's not absolutely needed there. Might conflict with the rule to catch errors as early as possible, but that's the trade-off.

Design - Run into an exception or avoid it?

2 Answers2