Обзор обработки исключений

Question

Мне интересно, правильно ли я обрабатываю исключения в моем коде, поэтому надеялся, что кто-нибудь подскажет мне мой код ниже.

 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");
        }
    }

мое исключение

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

    public CentralRepositoryException(string message){
    }
}

Answer 1

Есть несколько проблем с этим подходом:

• Возврат 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 !!!!!!!
    }
}

Answer 2

Это неправильное использование Dispose (). Если вы заметили, что написали:

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

Оба они относятся к одному и тому же объекту. Чтобы гарантировать, что вы утилизируете только один раз, это возможность повторного использования класса, объявляющего IDisposable для вызова dispose.

Это означает, что если вы создаете локальную переменную, вы делаете это в операторе using:

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

или явно распоряжаться:

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

Если вы создаете поле, вы также делаете свой класс одноразовым:

class MyDisposable : IDisposable
{
    private SomethingDisposable foo = new SomethingDisposable();

    void IDisposable.Dispose()
    {
        foo.Dispose();
    }
}

Если вы относитесь к таким IDisposables таким образом, ваша обработка исключений не будет путаться с вашим удалением.

Answer 3

Вы не должны просто повторно генерировать исключение, поскольку теряете трассировку стека. Я вижу, что вы используете их для удаления объектов, это должно быть в наконец-то блоке .

Если вы не можете по-настоящему обработать исключение, вы не должны использовать перехват, поскольку вы хотите использовать его для всплытия стека вызовов, чтобы его можно было исправить - вместо скрытый.