4

According to this MSDN article, you should not catch general exceptions. I'm sure there's a stackoverflow question dealing with that, and I understand why it's not a good practice but I just saw this example on another MSDN article today:

using System;
using System.IO;

class Test
{
    public static void Main()
    {
        try
        {
            using (StreamReader sr = new StreamReader("TestFile.txt"))
            {
                String line = sr.ReadToEnd();
                Console.WriteLine(line);
            }
        }
        catch (Exception e)
        {
            Console.WriteLine("The file could not be read:");
            Console.WriteLine(e.Message);
        }
    }
}

Is there any justification to catching a general exception in that example or is it just that they were lazy to write an example catching all the specific exceptions?

4

3 回答 3

2

In this case, this is an example and is a poor one at that, like many code examples on MSDN.

This should be catching an IO Exception instead of the base class.

The only place it would make sense to catch Exception is in a global exception handler for logging, provided you rethrow.

于 2012-10-12T09:31:01.127 回答
1

Rule "CA1031: Do not catch general exception types" is utterly useless. (not it's intent per se, but it's implementation and description)

In this specific case it may be feasible to catch the more specific IOException as it covers "most" of the exceptions the StreamReader constructor is documented to throw.

(oh wait, and then you also need to factor in all the stuff that could be thrown by ReadToEnd)

(oh, wait, maybe it's a legitimate exception catch case to have an ArgumentException for the case the passed string is empty -- that is, I want to catch that generically and not explicitly test for it.)

Except that:

  • The set of exceptions that possibly can be thrown from any function is

    1. often poorly documented
    2. potentially unbounded (there may lurk some OutOfMemoryException; bugs in the implementation that lead to other exceptions, etc.)
    3. largely irrelevant (modulo some bug catching with things like ArgumentNullException)

Given roughly the example given, all you care about is that reading the file failed - IFF it failed, you need to communicate that up the call chain.

To repeat: You want to communicate up the call chain that reading the file failed and why is only secondary.

When did reading (processing) the file fail: When any exception is thrown.

So what you really want to do is catch any exception, contextualize it with what you wanted to do (say, add the filename) and then return or rethrow something that indicates what went wrong and why it went wrong ( that would be the actual "inner" exception).

For this, the example is (nearly) spot on:

  • "Up the call chain" in this case is the user directly at the console
  • It "returns" the error as a string to be read by the "user", which is appropriate for a simple consle program.
  • It communicates what went wrong clearly. (It should include the filename passed to StreamReader though.)
  • It includes why it went wrong, namely the exception message. (It possibly should also include the call stack.)

By example:

Say that this example is slightly more complex and the file name is user provided. The the string being empty is then (a) a normal case and maybe (b) a user error that might not warrant special handling in the code, because the general handling provided -- that is, reporting that reading the file failed -- is totally enough.

By catching only IOException as recommended above, the user entering an empty filename would either crash the application or require an additional catch block that would essentially do the same thing as the other catch block: Report that reading the file failed and why.

Independent observations: Found this nice piece: http://www.codeproject.com/Articles/7557/Exception-Handling-in-C-with-the-quot-Do-Not-Catch where the author is certainly more constructive than my "utterly useless" statement but essentially has a similar conclusion (with a different, certainly valid, solution): (emph. added)

If we follow Microsoft's guidelines then we have to catch only those exceptions that could be triggered by the File.Copy method. If you look in the .NET Framework's Class Library, you'll see that this is UnauthorizedAccessException, (...). If you ask me, catching all these exceptions individually is not doable. All I wanted to know is whether or not a file copy failed or not. It would be sufficient to only capture IOException, ArgumentException and NotSupportedException (because all the other exceptions derive from these three exceptions) but to know this I have to read the documentation again. I really don't want to go through that much trouble to simply copy a file.

The guideline assumes that we know all the exceptions that can be thrown by the task executed. Copying a file is a very simple operation that is properly documented by Microsoft. But what if we're using some third-party tool? The stability of our unattended tool depends on the quality of the documentation of this tool. If this third-party forgets to mention one of the exceptions that their tool throws in the documentation, then our unattended application could stop prematurely.

于 2014-12-09T15:36:25.413 回答
0

Examples should be clear and simple. This is the example of clear and simple example. But we should not catch general exeption in our code.

于 2012-10-12T09:31:14.367 回答