CODEWITHSUNDEEP
c#.netexception
lyml
lyml

Reputation: 95

Is it good form to catch System.Exception when the reason for failure is unimportant?

I have a complicated programs which iterate over a bunch of input files does a lot of parsing and calculations and then outputs several other files.

void ParseFiles(string[] files, ProcessingDescription description) {
  foreach(string file in files) {
    try {
      DoComplicatedProcessing(file, description);
    }
    catch(Exception e) {
      LogFailure(file, e);
    }
  }
  DisplayResult();
}

My reason for catching System.Exception in this case is that this could fail for a number of reasons, bugs in the parsing/calculation, illegal values in the input files, permission problems even out-of-memory errors if my assumption that all necessary data could fit in memory turns out to be false.

But in the end I don't care why it failed. If the program fails for a specific file I want it to log that failure and continue with the next file.

At the end of the session a user might come back and see which files failed and what error message was thrown, sure in many cases the error message might not help the user but it's better than the program crashing on the first bad file.

I keep hearing "Never catch System.Exception" but I can't really see any other way of doing this.

Upvotes: 4

Views: 163

Answers (5)

Kit
Kit

Reputation: 21699

It is good form for your application to meet the requirements of solving the problem you are trying to solve, and if it is expedient and functionally correct to do it the way you're doing it, then do it.

Absolutes ignore context, and there is no such thing as best-practice -- only "best practice for this period of time for our context."

Upvotes: 1

Channs
Channs

Reputation: 2101

In general, I prefer not to throw exceptions (unless the application processing is forced to stop) since they cause a performance overhead. This MSDN article gives a good overview.

Based on how complicated your processing and since you only want to log failures, I would evaluate adding more 'sanity checks' wherever possible. Something like this:

void ParseFiles(string[] files, ProcessingDescription description) {
  foreach(string file in files) {
    if(!file.IsValid()) { //uses an extension method
      LogFailure(file, "Your Message"); //use the appropriate API
      continue;
    }

    try {    
      DoComplicatedProcessing(file, description);
    }
    catch(Exception e) {
      LogFailure(file, e);
    }
  }
  DisplayResult();
}

Upvotes: 1

David W
David W

Reputation: 10184

Seems to me this is a perfectly reasonable case where catching Exception makes sense, because it accomplishes what you need in your problem space. I get very cautious of universal rules like "Never do this, always do that..." A little bit of flexibility and common sense in those rules sure helps me. I think your code is just fine.

Upvotes: 2

Sergey Berezovskiy
Sergey Berezovskiy

Reputation: 236188

You should catch only those exceptions, which you could handle. In this case you can handle any exception, thus there is nothing wrong with this code.

Upvotes: 3

Davin Tryon
Davin Tryon

Reputation: 67296

Personally, I don't see anything wrong with this. You have clearly thought about the problem and have a solution that fits. Any time someone says "Never ..." in software development there is usually a situation were the reverse is true. I guess that is why others say things like, "It is best practice to ..." and "Usually ...". :)

If you are aware that by catching Exception you will loose granularity of the exception and that you might encounter strange edge cases (memory etc), then it is up to you whether you take on the risk.

Upvotes: 2

Related Questions