CODEWITHSUNDEEP
c#exception
ekkis
ekkis

Reputation: 10226

Try/Catch in every method of every class?

When we wrap a bunch of statements in a try/catch and one of these issues an exception, within the catch we have no way of knowing which of the statements caused the exception (the ex.stacktrace shows our current method (doit), its caller, its caller's caller, etc. but neither do1 or do2):

function doit() {
   try {
     do1();
     do2();
     [...]
   }
   catch (Exception ex) {
     // what failed?
   }
}

generally I've taken to wrapping all statements and rethrowing, sort of like:

private void do1() {
  try {
     // do whatever
  } catch(Exception e) {
     // write to my error log
     throw new Exception("do1: " + e.Message, e.InnerException);
  }
}

which leaves a trail of breadcrumbs in my log and makes the chain available for the upstream. The problem, of course, is that I have to wrap every method I write with this kind of code.

something tells me I'm being dumb about it. what is the right approach?

Upvotes: 5

Views: 16471

Answers (4)

Dave Cousineau
Dave Cousineau

Reputation: 13148

try and catch seem to me to be probably the single worst designed modern programming mechanism. We no longer have the ability to handle errors as we go; we must fail the entire procedure if a single exception occurs, unless we do something horrifying like try every statement individually. There is no longer the option of recovering, only failing as gracefully as possible.

The best pattern for this I have found so far is to wrap every user event in a try/catch (using a method, not an explicit try every time). Ex:

public static class Defines
{
   public static bool TryAction(Action pAction)
   {
      try { pAction(); return true; }
      catch(Exception exception) { PostException(exception); return false; }
   }
}

...

private void DoSomething(int pValue)
{
   ...
}

private void MyControl_MyEvent(object pSender, MyEventArgs pEventArgs)
{
   Defines.TryAction(() => DoSomething(pEventArgs.Data));
}

Beyond that, just try to write exception-less code. Only use explicit trys when you are very likely to get an exception and want to do a little more than just fail gracefully.

Upvotes: 2

M.Stramm
M.Stramm

Reputation: 1309

Ok, this is hard to get right because exception handling is a really really touchy subject and in the past people have fought religious wars over how to do this right.

First off: neither use an empty catch (try { ... } catch { ... }), nor catch(Exception ex). The sole purpose of Exception-derived classes is to give you rich information about the kind of exception which occured so you can do something meaningful in your exception handler (if a thread crashed restart it, if a db connection failed non-permanently try again, then fail, etc).

People tend to use a catch-all handler for the outermost part of their code to log uncaught exceptions and this is kind of OK, but in any case you should either prompt the user or rethrow the exception (using throw, not throw ex - there is a ton of discussion about this as well).

Basically you do not programmatically care about where the Exception occured at all. You can either handle it or you can't. If you can't handle it then you don't catch it.

Addendum: The most important reason for this "if you can do something about it, do so, otherwise dont you dare touch that exception" philosophy is that silently caught exceptions (whether logged or not) can lead to really hard-to-find bugs. Just pushing them out to a logfile might not be enough because in a live system you may not get the fully annotated stack trace (with line numbers and everything).

Addendum 2: Take for instance a textbox which expects an integer input. If the user supplies a string you cannot meaningfully process the input, maybe a conversion exception gets thrown, you catch that specific exception and reset the textbox to its old value and maybe inform the user about the erroneous input. Alternatively your program might either just die with an exception (bad design, you could recover from that exception) or silently go on showing the erroneous input, but still using the old value (bad design, the program is misleading).

Upvotes: 9

Brian Dishaw
Brian Dishaw

Reputation: 5825

If you are really sold on doing this (others have stated why this is bad already), use an Aspect Oriented Programming approach. This will make your life significantly easier and reduce the amount of code you end up writing and maintaining.

Take a look at PostSharp, it gives you a framework that allows you to decorate methods, classes, or namespaces with attributes that will generate this boilerplate error handling for you.

Upvotes: 4

robrich
robrich

Reputation: 13205

@Branko nailed it in the comments: the exception's stack trace shows the point where the exception was thrown, not where it was caught.

+1 for @ChaosPandion's comment: this is a very, very, very bad idea.

Upvotes: 3

Related Questions