Как выглядит код, когда вы не используете исключения для управления потоком? - PullRequest
8 голосов
/ 28 мая 2009

Я воспользовался советом, который видел в других ответах на вопросы о том, когда генерировать исключения, но теперь в моих API появился новый шум. Вместо вызова методов, заключенных в блоки try / catch (досадные исключения), у меня есть параметры аргумента с набором ошибок, которые могли произойти во время обработки. Я понимаю, почему упаковка всего в try / catch - это плохой способ контролировать поток приложения, но я редко вижу где-либо код, отражающий эту идею.

Вот почему все это кажется мне таким странным. Это практика, которая якобы является правильным способом кодирования, но я нигде не вижу этого. Кроме того, я не совсем понимаю, как относиться к клиентскому коду, когда происходит «плохое» поведение.

Вот фрагмент кода, который я взломал и который касается сохранения изображений, загруженных пользователями веб-приложения. Не волнуйтесь о деталях (это ужасно), просто посмотрите, как я добавил эти выходные параметры ко всему, чтобы получить сообщения об ошибках.

public void Save(UserAccount account, UserSubmittedFile file, out IList<ErrorMessage> errors)
{
    PictureData pictureData = _loader.GetPictureData(file, out errors);

    if(errors.Any())
    {
        return;
    }

    pictureData.For(account);

    _repo.Save(pictureData);
}

Это правильная идея? Я могу разумно ожидать, что предоставленный пользователем файл каким-то образом недействителен, поэтому я не должен выдавать исключение, однако я хотел бы знать, что не так с файлом, поэтому я выдаю сообщения об ошибках. Аналогично, любой клиент, который теперь использует этот метод сохранения, также захочет выяснить, что не так с общей операцией сохранения изображения.

У меня были другие идеи о возвращении некоторого объекта статуса, который содержал результат и дополнительные сообщения об ошибках, но это странно. Я знаю, что без параметров везде будет сложно поддерживать / рефакторинг / и т.д.

Я хотел бы получить некоторые советы по этому вопросу!

РЕДАКТИРОВАТЬ: Я думаю, что фрагмент представленных пользователем файлов может заставить людей задуматься об исключениях, возникающих при загрузке недопустимых изображений и других "серьезных" ошибок. Я думаю, что этот фрагмент кода является лучшей иллюстрацией того, где, как мне кажется, идея создания исключения не приветствуется.

С этим я просто сохраняю новую учетную запись пользователя. Я выполняю проверку состояния учетной записи пользователя, а затем попадаю в постоянное хранилище, чтобы узнать, было ли взято имя пользователя.

public UserAccount Create(UserAccount account, out IList<ErrorMessage> errors)
{
    errors = _modelValidator.Validate(account);

    if (errors.Any())
    {
        return null;
    }

    if (_userRepo.UsernameExists(account.Username))
    {
        errors.Add(new ErrorMessage("Username has already been registered."));
        return null;
    }

    account = _userRepo.CreateUserAccount(account);

    return account;
}

Должен ли я выдать какое-то исключение из проверки? Или я должен возвращать сообщения об ошибках?

Ответы [ 6 ]

9 голосов
/ 28 мая 2009

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

Возврат ошибок в выходных параметрах или возврат кодов состояния кажется немного неуклюжим. Иногда, сталкиваясь с этой ситуацией, я пытаюсь представить, как .NET Framework будет обрабатывать ошибки. Я не верю, что существует много методов платформы .NET, которые возвращают ошибки в выходных параметрах или возвращают коды состояния.

7 голосов
/ 28 мая 2009

По определению, «исключение» означает исключительное обстоятельство, от которого рутина не может восстановиться. В приведенном вами примере это выглядит так, что это означает, что изображение было недействительным / поврежденным / нечитаемым / и т.д. Это должно быть брошено и всплыть до самого верхнего слоя, и там решит, что делать с исключением. Само исключение содержит наиболее полную информацию о том, что пошло не так, что должно быть доступно на верхних уровнях.

Когда люди говорят, что вы не должны использовать исключения для управления потоком программ, они имеют в виду следующее: (например), если пользователь пытается создать учетную запись, но учетная запись уже существует, вы не должны создавать исключение AccountExistsException, а затем перехватывать его выше. в приложении, чтобы иметь возможность предоставить эту обратную связь пользователю, потому что учетная запись уже существует, не является исключительным случаем. Вы должны ожидать такой ситуации и обрабатывать ее как часть вашего обычного программного потока. Если вы не можете подключиться к базе данных, , что является исключительным случаем.

Часть проблемы с вашим примером регистрации пользователя заключается в том, что вы пытаетесь слишком много заключить в одну процедуру. Если ваш метод пытается сделать больше, чем одну вещь, то вы должны отслеживать состояние нескольких вещей (следовательно, вещи становятся ужасными, например списки сообщений об ошибках). В этом случае вы могли бы сделать следующее:

UsernameStatus result = CheckUsernameStatus(username);
if(result == UsernameStatus.Available)
{
    CreateUserAccount(username);
}
else
{
    //update UI with appropriate message
}

enum UsernameStatus
{
    Available=1,
    Taken=2,
    IllegalCharacters=3
}

Очевидно, что это упрощенный пример, но я надеюсь, что суть ясна: ваши подпрограммы должны пытаться делать только одно, и должны иметь ограниченную / предсказуемую область действия. Это облегчает остановку и перенаправление потока программы для решения различных ситуаций.

3 голосов
/ 28 мая 2009

В таких ситуациях, как у вас, я обычно вызываю вызывающее исключение. У меня есть немного другое представление об исключениях, может быть, чем у других: если метод не может сделать то, для чего он предназначен (т. Е. Что говорит имя метода: создать учетную запись пользователя), тогда он должен выдать исключение я: не делать то, что ты должен делать, это исключительное.

Для примера, который вы разместили, у меня будет что-то вроде:

public UserAccount Create(UserAccount account)
{
    if (_userRepo.UsernameExists(account.Username))
        throw new UserNameAlreadyExistsException("username is already in use.");
    else
        return _userRepo.CreateUserAccount(account);
}

Преимущество, по крайней мере для меня, заключается в том, что мой интерфейс тупой. Я просто пытаюсь / поймать любую функцию и в сообщении появляется сообщение об исключении, например:

try
{
    UserAccount newAccount = accountThingy.Create(account);
}
catch (UserNameAlreadyExistsException unaex)
{
    MessageBox.Show(unaex.Message);
    return; // or do whatever here to cancel proceeding
}
catch (SomeOtherCustomException socex)
{
    MessageBox.Show(socex.Message);
    return; // or do whatever here to cancel proceeding
}
// If this is as high up as an exception in the app should bubble up to, 
// I'll catch Exception here too

По стилю это похоже на многие методы System.IO (например, http://msdn.microsoft.com/en-us/library/d62kzs03.aspx).

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

3 голосов
/ 28 мая 2009

Я думаю, что это неправильный подход. Да, очень вероятно, что вы будете иногда получать недействительные изображения. Но это все еще исключительный сценарий. По моему мнению, исключения являются правильным выбором здесь.

0 голосов
/ 28 мая 2009

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

В языке, подобном C #, который предлагает структурированную обработку исключений, идея состоит в том, чтобы позволить «исключительным» случаям в вашем коде идентифицироваться, распространяться и со временем обрабатываться. Обработка, как правило, остается на высшем уровне вашего приложения (то есть клиент Windows с пользовательским интерфейсом и диалогами ошибок, веб-сайт со страницами ошибок, средство ведения журнала в цикле сообщений фоновой службы и т. Д.) В отличие от Java, который использует проверенная обработка исключений, C # не требует от вас специально обрабатывать каждое исключение, которое может пройти через ваши методы. Напротив, попытка сделать это, несомненно, приведет к серьезным узким местам в производительности, поскольку отлов, обработка и, возможно, повторное создание исключений - дорогостоящее дело.

Общая идея с исключениями в C # заключается в том, что если они случаются ... и я подчеркиваю , если , потому что они называются исключениями из-за того факта, что во время нормальной работы вы не должно возникать никаких исключительных условий , ... если они случаются, у вас есть инструменты для безопасного и аккуратного восстановления и представления пользователя (если он есть) с уведомлением о сбое приложений и возможных вариантах разрешения.

В большинстве случаев хорошо написанное приложение на C # не будет иметь столько блоков try / catch в основной бизнес-логике, и будет иметь гораздо больше вариантов try / finally или, что еще лучше, с использованием блоков. Для большинства кода проблема в ответе на исключение состоит в том, чтобы нормально восстановиться путем освобождения ресурсов, блокировок и т. Д. И разрешения продолжения исключения. В вашем коде более высокого уровня, обычно во внешнем цикле обработки сообщений приложения или в стандартном обработчике событий для систем, таких как ASP.NET, вы в конечном итоге выполните структурированную обработку с помощью try / catch, возможно, с несколькими предложениями catch для обработки конкретных ошибок, которые требуют уникальной обработки.

Если вы правильно обрабатываете исключения и строите код, который использует исключения соответствующим образом, вам не нужно беспокоиться о большом количестве блоков try / catch / finally, кодов возврата или сложных извлечений методов с большим количеством ref и out параметры. Вы должны увидеть код, похожий на этот:

public void ClientAppMessageLoop()
{
    bool running = true;
    while (running)
    {
        object inputData = GetInputFromUser();
        try
        {
            ServiceLevelMethod(inputData);
        }
        catch (Exception ex)
        {
            // Error occurred, notify user and let them recover
        }
    }
}

// ...

public void ServiceLevelMethod(object someinput)
{
    using (SomeComponentThatsDisposable blah = new SomeComponentThatsDisposable())
    {
        blah.PerformSomeActionThatMayFail(someinput);
    } // Dispose() method on SomeComponentThatsDisposable is called here, critical resource freed regardless of exception
}

// ...

public class SomeComponentThatsDisposable: IDosposable
{
    public void PErformSomeActionThatMayFail(object someinput)
    {
        // Get some critical resource here...

        // OOPS: We forgot to check if someinput is null below, NullReferenceException!
        int hash = someinput.GetHashCode();
        Debug.WriteLine(hash);
    }

    public void Dispose()
    {
        GC.SuppressFinalize(this);

        // Clean up critical resource if its not null here!
    }
}

Следуя вышеприведенной парадигме, вы не имеете много грязного кода try / catch повсюду, но вы все еще «защищены» от исключений, которые в противном случае прерывают ваш обычный программный поток и выливаются на уровень обработки исключений более высокого уровня код.

EDIT:

Хорошей статьей, которая охватывает предполагаемое использование исключений и почему исключения не проверяются в C #, является следующее интервью с Андерсом Хейлсбергом, главным архитектором языка C #:

http://www.artima.com/intv/handcuffsP.html

РЕДАКТИРОВАТЬ 2:

Чтобы предоставить лучший пример, который работает с кодом, который вы разместили, возможно, следующее будет более полезным. Я угадываю некоторые имена и делаю вещи одним из способов, с которыми я столкнулся при внедрении сервисов ... так что простите мне любую лицензию, которую я беру:

public PictureDataService: IPictureDataService
{
  public PictureDataService(RepositoryFactory repositoryFactory, LoaderFactory loaderFactory)
  {
     _repositoryFactory = repositoryFactory;
     _loaderFactory = loaderFactory;
  }

  private readonly RepositoryFactory _repositoryFactory;
  private readonly LoaderFactory _loaderFactory;
  private PictureDataRepository _repo;
  private PictureDataLoader _loader;

  public void Save(UserAccount account, UserSubmittedFile file)
  {
    #region Validation
    if (account == null) throw new ArgumentNullException("account");
    if (file == null) throw new ArgumentNullException("file");
    #endregion

    using (PictureDataRepository repo = getRepository())
    using (PictureDataLoader loader = getLoader())
    {
      PictureData pictureData = loader.GetPictureData(file);
      pictureData.For(account);
      repo.Save(pictureData);
    } // Any exceptions cause repo and loader .Dispose() methods 
      // to be called, cleaning up their resources...the exception
      // bubbles up to the client
  }

  private PictureDataRepository getRepository()
  {
    if (_repo == null)
    {
      _repo = _repositoryFactory.GetPictureDataRepository();
    }

    return _repo;
  }

  private PictureDataLoader getLoader()
  {
    if (_loader == null)
    {
        _loader = _loaderFactory.GetPictureDataLoader();
    }

    return _loader;
  }
}

public class PictureDataRepository: IDisposable
{
  public PictureDataRepository(ConnectionFactory connectionFactory)
  {
  }

  private readonly ConnectionFactory _connectionFactory;
  private Connection _connection;

  // ... repository implementation ...

  public void Dispose()
  {
    GC.SuppressFinalize(this);

    _connection.Close();
    _connection = null; // 'detatch' from this object so GC can clean it up faster
  }
}

public class PictureDataLoader: IDisposable
{
  // ... Similar implementation as PictureDataRepository ...
}
0 голосов
/ 28 мая 2009

Я бы тоже допустил исключения, но в зависимости от вашей темы вы ищете альтернативу. Почему бы не включить информацию о состоянии или ошибках в объект PictureData. Затем вы можете просто вернуть объект с ошибками, а остальные вещи оставить пустыми. Просто предложение, но вы в значительной степени делаете именно то, что были сделаны для решения исключений:)

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...