13

I've read many blogs/articles/book chapters about proper exception handling and still this topic is not clear to me. I will try to illustrate my question with following example.

Consider the class method that has following requirements:

  1. receive list of file paths as parameter
  2. read the file content for each file or skip if there is any problem trying to do that
  3. return list of objects representing file content

So the specs are straightforward and here is how I can start coding:

    public class FileContent
    {
        public string FilePath { get; set; }
        public byte[] Content { get; set; }

        public FileContent(string filePath, byte[] content)
        {
            this.FilePath = filePath;
            this.Content = content;
        }
    }

    static List<FileContent> GetFileContents(List<string> paths)
    {
        var resultList = new List<FileContent>();

        foreach (var path in paths)
        {
            // open file pointed by "path"
            // read file to FileContent object
            // add FileContent to resultList
            // close file
        }

        return resultList;
    }

Now note that the 2. from the specs says that method should "skip any file which content can't be read for some reason". So there could be many different reasons for this to happen (eg. file not existing, file access denied due to lack of security permissions, file being locked and in use by some other application etc...) but the point is that I should not care what the reason is, I just want to read file's content if possible or skip the file if not. I don't care what the error is...

So how to properly implement this method then?

OK the first rule of proper exception handling is never catch general Exception. So this code is not good then:

    static List<FileContent> GetFileContents(List<string> paths)
    {
        var resultList = new List<FileContent>();

        foreach (var path in paths)
        {
            try
            {
                using (FileStream stream = File.Open(path, FileMode.Open))
                using (BinaryReader reader = new BinaryReader(stream))
                {
                    int fileLength = (int)stream.Length;
                    byte[] buffer = new byte[fileLength];
                    reader.Read(buffer, 0, fileLength);

                    resultList.Add(new FileContent(path, buffer));
                }
            }
            catch (Exception ex)
            {
                // this file can't be read, do nothing... just skip the file
            }
        }

        return resultList;
    }

The next rule of proper exception handlig says: catch only specific exceptions you can handle. Well I do not I care about handling any specific exceptions that can be thrown, I just want to check if file can be read or not. How can I do that in a proper, the best-practice way?

4

7 回答 7

6

尽管捕获和吞下非特定异常通常不被认为是一种好的做法，但风险往往被夸大了。

毕竟，ASP.NET 将捕获在处理请求期间抛出的非特定异常，并将其包装在 HttpUnhandledException 中后，将重定向到错误页面并愉快地继续前进。

在您的情况下，如果您想遵守该指南，您需要一份可以抛出的所有异常的完整列表。我相信以下列表是完整的：

UnauthorizedAccessException IOException FileNotFoundException DirectoryNotFoundException PathTooLongException NotSupportedException（路径格式无效）。 SecurityException ArgumentException

您可能不想捕获 SecurityException 或 ArgumentException，而其他几个都派生自，因此IOException您可能想要捕获IOException和。NotSupportedExceptionUnauthorizedAccessException

于 2013-11-07T15:41:52.157 回答
5

Your requirements are clear - skip files that cannot be read. So what is the problem with the general exception handler? It allows you to perform your task in a manner that is easy, clean, readable, scalable and maintainable.

If at any future date you want to handle the multiple possible exceptions differently, you can just add above the general exception the catch for the specific one(s).

So you'd rather see the below code? Note, that if you add more code to handle the reading of files, you must add any new exceptions to this list. All this to do nothing?

try
{
    // find, open, read files
}
catch(FileNotFoundException) { }
catch(AccessViolation) { }
catch(...) { }
catch(...) { }
catch(...) { }
catch(...) { }
catch(...) { }
catch(...) { }

Conventions are guidelines and great to try to adhere to to create good code - but do not over-complicate code just to maintain some odd sense of proper etiquette.

To me, proper etiquette is to not talk in bathrooms - ever. But when the boss says hello to you in there, you say hello back. So if you don't care to handle multiple exceptions differently, you don't need to catch each.

Edit: So I recommend the following

try
{
    // find, open, read files
}
catch { } // Ignore any and all exceptions

The above tells me to not care which exception is thrown. By not specifying an exception, even just System.Exception, I've allowed .NET to default to it. So the below is the same exact code.

try
{
    // find, open, read files
}
catch(Exception) { } // Ignore any and all exceptions

Or if you're going to log it at least:

try
{
    // find, open, read files
}
catch(Exception ex) { Logger.Log(ex); }  // Log any and all exceptions
于 2013-11-07T14:58:36.640 回答
2

Something you can consider in this instance is that between the FileNotFoundException, which you can't catch because there are too many of them, and the most general Exception, there is still the layer IOException.

In general you will try to catch your exceptions as specific as possible, but especially if you are catching the exceptions without actually using them to throw an error, you might as well catch a group of exceptions. Even then however you will try to make it as specific as possible

于 2013-11-07T14:59:26.450 回答
2

我对这个问题的解决方案通常是基于可能的例外的数量。如果只有几个，我会为每个指定 catch 块。如果有很多可能，我会捕获所有异常。强迫开发人员总是捕捉特定的异常会导致一些非常丑陋的代码。

于 2013-11-07T14:51:15.587 回答
2

您在一种方法中混合了不同的操作，更改代码将使您更容易提问： 

static List<FileContent> GetFileContents(List<string> paths)
{
    var resultList = new List<FileContent>();

    foreach (var path in paths)
    {
          if (CanReadFile(path){
                resultList.Add(new FileContent(path, buffer));
          }
    return resultList;
}

static bool CanReadFile(string Path){
     try{
         using (FileStream stream = File.Open(path, FileMode.Open))
            using (BinaryReader reader = new BinaryReader(stream))
            {
                int fileLength = (int)stream.Length;
                byte[] buffer = new byte[fileLength];
                reader.Read(buffer, 0, fileLength);
            }
     }catch(Exception){ //I do not care what when wrong, error when reading from file
         return false;
     }
     return true;
}

这样 CanReadFile 隐藏了您检查的实现。您唯一需要考虑的是 CanReadFile 方法是否正确，或者它是否需要错误处理。

于 2013-11-07T15:06:00.300 回答
1

在我看来，将异常分为三种类型。首先是您期望并知道如何恢复的异常。其次是您知道可以在运行时避免的异常。第三是您不希望在运行时发生但无法避免或无法实际处理的那些。

处理第一种类型，这些是对您的特定抽象级别有效的异常类，它们代表在该级别恢复的有效业务案例（在您的情况下，忽略。）

应该避免第二类异常——不要偷懒。第三类异常应该放过……你需要确保你知道如何处理一个问题，否则你可能会让你的应用程序处于混乱或无效的状态。

正如其他人所说，您可以通过向现有 try 块添加更多 catch 块来处理多个异常，它们按照它们出现的顺序进行评估，因此如果您必须处理源自其他异常的异常，您也可以处理这些异常，请使用更多先说具体的。

于 2013-11-14T13:50:04.640 回答
1

这重复了所说的内容，但希望以某种方式让您更好地理解。

您在“跳过由于某种原因无法读取内容的任何文件”中出现逻辑错误。

如果该原因是您的代码中的错误，您不想跳过它。
您只想跳过具有文件相关错误的文件。
如果 FileContent 中的 ctor 抛出错误怎么办？

例外是昂贵的。
我会测试 FileExists （并且仍然捕获异常）
并且我同意 Joe
Come 在 MSDN 上列出的异常有如何捕获各种异常的明确示例

于 2013-11-07T21:10:58.093 回答