3

I have this code block:

try
{
    int QuestionAnswerID = 0;

    // code block which assign value to QuestionAnswerID 

    item.QuestionAnswerID = QuestionAnswerID;
}
catch (NullReferenceException)
{
    item.QuestionAnswerID = -999;
}

This runs in a loop and this surely run into catch block 2-3 times within the loop. This code does exactly what I wanted but just wanted to know whether it is a bad practice to handle a known problem using try-catch block.

Will it be more efficient if I use if statement/s to identify null value before throwing excenption?

kapz
  • 437
  • 1
  • 7
  • 15
  • 7
    try-catch will be slower than using `if`, also it looks cleaner to use if than try-catch. If it's not real exception, don't use try-catch – Kamil Budziewski Oct 22 '13 at 08:43
  • 5
    Never write code to catch a NullReferenceException. They indicate a bug in your program. Fix the bug instead. – Kris Vandermotten Oct 22 '13 at 08:45
  • and In catch you're setting `item.QuestionAnswerID` what if item is `null`? – Sriram Sakthivel Oct 22 '13 at 08:46
  • The only `NullreferenceExcpetion'` that can occur here, is when `item` is null, so it will be thrown again in the `catch` block. – Rik Oct 22 '13 at 08:46
  • 1
    If you know why you're potentially creating and trying to use a null reference, you should check and handle at that point. If you don't know why, but are just fixing the symptoms with a catch block, you will be much better off trying to get to the root of the problem. – Baldrick Oct 22 '13 at 08:47
  • @KrisVandermotten, this is simply not true. It is common (though not necessarily good) practice to return null to indicate some sort of error or state. Just treating the value as valid, in order to catch the null exception, will result in the exception being thrown, even though there's no bug. – David Arno Oct 22 '13 at 08:48
  • Read this: http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx and this: http://blogs.msdn.com/b/kcwalina/archive/2007/01/30/exceptionhierarchies.aspx for a better general understanding of exceptions. – Kris Vandermotten Oct 22 '13 at 08:49
  • possible duplicate of [Why is try {...} finally {...} good; try {...} catch{} bad?](http://stackoverflow.com/questions/128818/why-is-try-finally-good-try-catch-bad) – Ramesh Rajendran Oct 22 '13 at 08:49
  • @DavidArno In addition to what I said before: never use the throw of a NullReferenceException as a control flow mechanism, as it may hide real bugs in your code. Furthermore, an explicit `if` at the location of the dereference will (in general) be easier to understand and maintain, than a catch block several lines further below. – Kris Vandermotten Oct 22 '13 at 08:55
  • @KrisVandermotten, that I completely agree with (as that is my answer :) – David Arno Oct 22 '13 at 08:57

3 Answers3

11

Yes, this is bad practice, because throwing and catching exceptions is quite expensive, and exceptions should not be used for regular operation, only for error handling.

The preferred way to go about this, is to check whether the object is null yourself and handle that case appropriately.

Rik
  • 28,507
  • 14
  • 48
  • 67
  • 2
    "They" always _told_ us that this is expensive. Maybe it has become an Urban Legend like "HTTPS is way more server-resource intensive than HTTP"?!? – Uwe Keim Oct 22 '13 at 08:47
  • 1
    Either way, they are SURELY more expensive than if (obj != null) ... :) But even if they were not, exceptions are meant to handle program behaviour that falls OUTSIDE the normal bounds of the program flow of execution. It makes code very confusing and unmaintainable if they are used indiscriminately. – Baldrick Oct 22 '13 at 08:48
  • @UweKeim Even then, exceptions are not designed to handle regular flow and should be avoided in this case. a simple `if` condition describes the intended behavior more accurately. – Rik Oct 22 '13 at 08:58
  • We all agree it's bad practice. And in your comment you gave the reason: they're not meant to handle regular control flow. But in your answer you argue that the reason is that exceptions supposedly are expensive. Even if they are, that would be the wrong reason. – Kris Vandermotten Oct 22 '13 at 09:06
  • @KrisVandermotten I have added this to my answer. – Rik Oct 22 '13 at 09:09
  • Oops... be careful with phrases such as "they are called exceptions for a reason". See my comment to Alex Key's answer: http://stackoverflow.com/questions/19513023/use-try-catch-to-handle-seen-errors-is-it-bad/19513085#comment28947223_19513085 – Kris Vandermotten Oct 22 '13 at 09:16
2

Whether it is more "expensive" (ie slower) to use try/catch should be seem as less important here than code reliability. What you are effectively doing with the code is:

block of code where something may go wrong

    various things occur

    specific thing that you have anticipated might cause an error occurs

    various things occur

    set return state to valid value

end of block where something may go wrong

if something went wrong

    set return state to invalid value

end if

return return state

You end up with a block of code where something might go wrong at some stage. You design it around an expected problem, but other problems may occur and you'll hide them. By testing for a null at the specific point where you expect the problem to occur, by using if (specific thing == null), then you avoid masking potential problems that you hadn't anticipated.

David Arno
  • 42,717
  • 16
  • 86
  • 131
1

Try catches are relatively expensive operations and should only be used in "exceptional circumstances" they should not be used in situations that you are planning for.

EDIT:

A better worded example of what I meant:

Exceptions should not be used to change the flow of a program as part of ordinary execution. Exceptions should only be used to report and handle error conditions.

http://msdn.microsoft.com/en-us/library/ms173163.aspx

Alex KeySmith
  • 16,657
  • 11
  • 74
  • 152
  • This is simply not true. Read this: http://blogs.msdn.com/b/kcwalina/archive/2008/07/17/exceptionalerror.aspx. Also take a look at http://blogs.msdn.com/b/kcwalina/archive/2007/01/30/exceptionhierarchies.aspx and http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx for a better understanding of exceptions. – Kris Vandermotten Oct 22 '13 at 09:11
  • Interesting links. Especially around that what is considered an exception varies greatly upon the context of the code. But you'd agree in this context an exception is inappropriate as it's a known condition and not error handling? I've updated my explanation of what I meant. – Alex KeySmith Oct 22 '13 at 12:11