异常处理审查

本文关键字：审查 异常处理 | 更新日期: 2023-09-27 18:11:09

我想知道我是否在我的代码中正确处理异常，所以希望有人能给我一些关于我下面的代码的想法

 public IEnumerable<Job> GetAll() {
        try {
            return this._context.Jobs;
        } catch (SqlException ex) {
            //error so dispose of context
            this.Dispose();
            //wrap and rethrow back to caller
            throw new CentralRepositoryException("Error getting all jobs", ex);
        }
    }

这个方法是我的业务逻辑的一部分，并调用上面的

方法
    public IEnumerable<Job> GetAllJobs() {
        try {
            return this._jobsRepository.GetAll();
        } catch (CentralRepositoryException ex) {
            //logging code to go here
            //throw back to caller
            throw;
        } catch (Exception ex) {
            this._jobsRepository.Dispose();
            //logging code to go here
            //throw simple exception to caller
            throw new CentralRepositoryException("A general exception has occurred");
        }
    }

my custom exception 

public class CentralRepositoryException : System.Exception {
    public CentralRepositoryException(string message, Exception innerException)
        : base(message, innerException) {
    }
    public CentralRepositoryException(string message){
    }
}

异常处理审查

这种方法有几个问题:

  • 返回IEnumerable<T>将导致代码执行延迟。即使您返回IEnumerable<T>，在代码中使用this._jobsRepository.GetAll().ToList();
  • 延迟执行导致错误处理程序根本不被调用，因为代码将在客户端上下文中运行延迟(即无论您在哪里使用它)
  • 将所有IDisposable对象包装在using块中

所以另一个选项是:

public IEnumerable<Job> GetAllJobs() {
    try {
        using(var jobsRepository = new JobsRepository()) // !!! Use Dependency Injection, etc
        {
              return jobsRepository .GetAll().ToList(); // !! NOTE: ToList() avoids delayed execution
        }
    } catch (CentralRepositoryException ex) {
        //logging code to go here
        //throw back to caller
        throw;
    } catch (Exception ex) {
        //logging code to go here
        //throw simple exception to caller
        throw new CentralRepositoryException("A general exception has occurred", ex); // !!!!!! INCLUDE THE ORIGINAL ERROR !!!!!!!
    }
}

您不应该在丢失堆栈跟踪时简单地重新抛出异常。我可以看到你正在使用它们来处理对象，这应该在finally块中。

除非你可以真正处理异常，否则你不应该使用catch，因为你希望在调用堆栈中冒泡，这样它就可以被修复，而不是被隐藏。

这不是Dispose()的正确用法。如果你注意到你最后写了:

this.Dispose(); 
this._jobsRepository.Dispose();

这两个都指向同一个对象。为了确保您只dispose一次，它是声明IDisposable的类调用dispose的可重构性。

这意味着如果你创建一个局部变量，你可以在using语句中这样做:

using(SomethingDisposable foo = new SomethingDisposable())
{
    //...
}

或显式dispose:

SomethingDisposable foo = new SomethingDisposable();
try
{
    //...
}
finally
{
    ((IDisposable)foo).Dispose();
}

如果你创建了一个字段，那么你的类也是一次性的:

class MyDisposable : IDisposable
{
    private SomethingDisposable foo = new SomethingDisposable();
    void IDisposable.Dispose()
    {
        foo.Dispose();
    }
}

如果你以这种方式处理你的IDisposables，那么你的异常处理将不会与你的处置混淆。

相关文章：