Бросать исключение правильного типа

Question

В моем коде часто бывают такие ситуации:

public void MyMethod(string data)
{
    AnotherClass objectOfAnotherClass = GetObject(data);
    if (objectOfAnotherClass == null)
        throw new WhatExceptionType1("objectOfAnotherClass is null.");

    if (objectOfAnotherClass.SomeProperty < 0)
        throw new WhatExceptionType2("SomeProperty must not be negative.");
}

Представьте себе, что GetObject использует некоторые внешние библиотеки, которые не находятся под моим контролем, и что эта библиотека возвращает null, если не существует объекта для data, и считает отрицательное SomeProperty допустимым состоянием и, следовательно, не не исключение. Далее представьте, что MyMethod не может работать без objectOfAnotherClass и не имеет смысла с отрицательным SomeProperty.

Каковы правильные исключения для WhatExceptionType1/2, чтобы бросить в этой ситуации?

В основном я имел в виду четыре варианта:

• 1) InvalidOperationException, поскольку MyMethod не имеет смысла в условиях, описанных выше. С другой стороны, руководящие принципы (и Intellisense в VS) также говорят, что InvalidOperationException должно быть выброшено, если объект, к которому принадлежит метод, находится в недопустимом состоянии. Теперь сам объект не находится в недопустимом состоянии. Вместо этого входной параметр data и некоторые другие операции, основанные на этом параметре, приводят к ситуации, когда MyMethod больше не может работать.

• 2) ArgumentException, поскольку существуют значения для data, с которыми метод может работать, а другие значения, с которыми он не может работать. Но я не могу проверить это путем проверки data в одиночку, мне нужно вызвать другие операции, прежде чем я решу.

• 3) Exception, потому что я не знаю, какой другой тип исключения использовать, и потому что все другие предопределенные исключения кажутся слишком специализированными и не соответствуют моей ситуации.

• 4) MyCustomException (мой собственный тип исключения, полученный из Exception). Кажется, это всегда вариант, но я беспокоюсь, что мне придется определить множество специальных классов исключений для многих различных состояний ошибки, когда я начну следовать этому шаблону.

Есть ли другие и лучшие варианты? Какие аргументы за или против этих вариантов?

Спасибо за отзыв заранее!

Answer 1

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

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

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

Answer 2

Будьте осторожны при использовании встроенных типов исключений ... они имеют очень специфические значения для .NET Framework, и если вы не используете его для точно того же значения, лучше свернуть свой собственный(+1 к Джону Сондерсу за Serializeable).

InvalidOperationException имеет значение:

Исключение, которое выдается, когда вызов метода недопустим для текущего состояния объекта.

Например, если вы позвоните SqlConnection.Open(), вы получите InvalidOperationException, если вы не указали источник данных.InvalidOperationException не подходит для вашего сценария.

ArgumentException тоже не подходит.Ошибка при создании objectOfAnotherClass может не иметь ничего общего с передачей неверных данных. Предположим, это имя файла, но GetObject() не имеет прав на чтение файла.Поскольку метод написан, нет способа узнать, почему произошел сбой вызова GetObject(), и лучшее, что вы можете сказать, это то, что возвращаемый объект был нулевым или недействительным.

Exception - просто плохая идея,в общем ... это не дает вызывающей стороне абсолютно никакого представления, почему метод не смог создать объект.(Впрочем, иметь только catch (Exception ex) {..} - тоже плохая идея)

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

ObjectCreateException:   // The call to GetObject() returned null<br />
InvalidObjectException:  // The object returned by GetObject() is invalid 
                         // (because the property < 0)

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

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

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

Команда разработчиков .NET решила, что ApplicationException не имеет никакого реального значения, и устарела, но пурист во мне всегдапонравилось, поэтому оно сохраняется в моем коде.Здесь, однако, это действительно не добавляет никакой ценности и только увеличивает глубину наследственной наследственности.Так что не стесняйтесь наследовать напрямую от Exception.

/// <summary>
/// A general, base error for GS3 applications </summary>
[Serializable]
public class Gs3Exception : ApplicationException {


    /// <summary>
    /// Initializes a new instance of the <see cref="Gs3Exception"/> class </summary>
    public Gs3Exception() {}


    /// <summary>
    /// Initializes a new instance of the <see cref="Gs3Exception"/> class </summary>
    /// <param name="message">A brief, descriptive message about the error </param>
    public Gs3Exception(string message) : base(message) {}


    /// <summary>
    /// Initializes a new instance of the <see cref="Gs3Exception"/> class 
    /// when deserializing </summary>
    /// <param name="info">The object that holds the serialized object data </param>
    /// <param name="context">The contextual information about the source or
    ///  destination.</param>
    public Gs3Exception(SerializationInfo info, StreamingContext context) : base(info, context) { }


    /// <summary>
    /// Initializes a new instance of the <see cref="Gs3Exception"/> class
    /// with a message and inner exception </summary>
    /// <param name="Message">A brief, descriptive message about the error </param>
    /// <param name="exc">The exception that triggered the failure </param>
    public Gs3Exception(string Message, Exception exc) : base(Message, exc) { }


}


/// <summary>
/// An object queried in an request was not found </summary>
[Serializable]
public class ObjectNotFoundException : Gs3Application {

    private string objectName = string.Empty;

    /// <summary>
    /// Initializes a new instance of the <see cref="ObjectNotFoundException"/> class </summary>
    public ObjectNotFoundException() {}


    /// <summary>
    /// Initializes a new instance of the <see cref="ObjectNotFoundException"/> class </summary>
    /// <param name="message">A brief, descriptive message about the error</param>
    public ObjectNotFoundException(string message) : base(message) {}


    /// <summary>
    /// Initializes a new instance of the <see cref="ObjectNotFoundException"/> class </summary>
    /// <param name="ObjectName">Name of the object not found </param>
    /// <param name="message">A brief, descriptive message about the error </param>
    public ObjectNotFoundException(string ObjectName, string message) : this(message) {
        this.ObjectName = ObjectName;
    }


    /// <summary>
    /// Initializes a new instance of the <see cref="ObjectNotFoundException"/> class.
    /// This method is used during deserialization to retrieve properties from 
    /// the serialized data. </summary>
    /// <param name="info">The object that holds the serialized object data.</param>
    /// <param name="context">The contextual information about the source or
    /// destination.</param>
    public ObjectNotFoundException(SerializationInfo info, StreamingContext context) : base(info, context) {
        if (null != info) {
            this.objectName = info.GetString("objectName");
        }
    }


    /// <summary>
    /// When serializing, sets the <see cref="T:System.Runtime.Serialization.SerializationInfo"/> 
    /// with information about the exception. </summary>
    /// <param name="info">The <see cref="T:System.Runtime.Serialization.SerializationInfo"/> that holds 
    /// the serialized object data about the exception being thrown.</param>
    /// <param name="context">The <see cref="T:System.Runtime.Serialization.StreamingContext"/> that contains contextual information about the source or destination.</param>
    /// <exception cref="T:System.ArgumentNullException">
    /// The <paramref name="info"/> parameter is a null reference (Nothing in Visual Basic) </exception>
    /// <PermissionSet>
    ///     <IPermission class="System.Security.Permissions.FileIOPermission, mscorlib, Version=2.0.3600.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" version="1" Read="*AllFiles*" PathDiscovery="*AllFiles*"/>
    ///     <IPermission class="System.Security.Permissions.SecurityPermission, mscorlib, Version=2.0.3600.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" version="1" Flags="SerializationFormatter"/>
    /// </PermissionSet>
    [SecurityPermissionAttribute(SecurityAction.LinkDemand, Flags=SecurityPermissionFlag.SerializationFormatter)]
    public override void GetObjectData(SerializationInfo info, StreamingContext context) {

        base.GetObjectData(info, context);

        //  'info' guaranteed to be non-null (base.GetObjectData() will throw an ArugmentNullException if it is)
        info.AddValue("objectName", this.objectName);

    }


    /// <summary>
    /// Gets or sets the name of the object not found </summary>
    /// <value>The name of the object </value>
    public string ObjectName {
        get { return objectName; }
        set { objectName = value; }
    }

}

PS: Прежде чем кто-либо позвонит мне, причина для базы Gs3Exception, которая добавляет не больше значения, чем ApplicationException, заключается в том, чтоБлок приложения для обработки исключений в корпоративной библиотеке ... имея базовое исключение на уровне приложения, мы можем создавать общие политики ведения журналов для исключений, создаваемых непосредственно нашим кодом.

Answer 3

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

Вторым является поведение GetObject ().Видимо, автор спроектировал его так, чтобы он был не исключительным , и ожидал, что он вернет ноль.Гораздо более сильным контрактом будут два метода, TryGetObject () и GetObject (), последний из которых выбрасывает.Или, чаще, дополнительный аргумент для GetObject () с именем throwOnFailure .Но этого не произошло, null возвращается и нормально.

Вы нарушаете этот контракт здесь.Вы пытаетесь превратить его из не исключительного в исключительное, но не понимаете, что не так.Самое лучшее, что нужно сделать, - это не изменять этот контракт и оставлять его на усмотрение вашего метода.В конце концов, это тот, кто знает, что делает GetObject ().Измените имя метода, используйте слово «Try» и верните bool.

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

Answer 4

Мой голос был бы ArgumentException по крайней мере в первом случае, если не оба;ArgumentExceptions должен быть брошен, заключить в кавычки, «когда один из аргументов, предоставленных методу, недопустим».Если MyMethod не может использовать аргумент data для создания действительного экземпляра AnotherClass, как ожидается MyMethod, этот аргумент недопустим для использования в MyMethod.

Следует понимать, что если вы не планируете перехватывать исключения разных типови обрабатывать их по-разному, какое именно исключение выдается на самом деле не имеет значения.Некоторые исключения (например, ArgumentNullException) создают пользовательское сообщение на основе очень небольшого количества информации, поэтому их легко настроить и локализовать, другие (например, SqlExceptions) содержат более конкретные данные о том, что пошло не так, но все это излишне, если все, что вы хотите, этовыбросить исключение, говорящее «Ой!».

Answer 5

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

Ошибка: с тупой головой , если вы можете сказать, что для правильного ввода name вы знаете, что GetObject вернет объект, который имеет смысл для вашего метода. Другими словами, единственной причиной этих исключений являются ошибки в коде, вызывающем MyMethod. В этом случае не имеет значения, какой тип исключения вы используете, потому что вы все равно никогда его не увидите в рабочей среде - ArgumentException (если проблема была с name) или InvalidOperationException (если проблема была с состоянием объекта, определяющего MyMethod), было бы хорошим выбором в этой ситуации, но конкретный тип исключения не должен быть задокументирован (иначе он станет частью API).

Ошибка досадная , если GetObject предсказуемо по отношению к name (т. Е. Для данного name вы либо всегда получите действительный объект, либо никогда не получите действительный объект), но name основано на пользовательском вводе (или конфигурации). В этом случае вы вообще не хотите создавать исключение; Вы хотите написать метод TryMyMethod, чтобы вызывающему коду не приходилось перехватывать исключение, чтобы справиться с неисключительной ситуацией.

Ошибка экзогенная , если поведение GetObject непредсказуемо по отношению к name: некоторое внешнее влияние может привести к тому, что name станет действительным или недействительным. В этом случае вам необходимо выбрать определенный тип исключения и задокументировать его как часть API. ArgumentException не будет хорошим выбором для экзогенного исключения; ни один не будет InvalidOperationException. Вы должны выбрать что-то более похожее на FileNotFoundException, более описательное для основной проблемы (независимо от того, что GetObject делает).

Answer 6

Как насчет использования ObjectNotFoundException. Это бы правильно описало ситуацию.

Answer 7

Я знаю, что этот ответ слишком, слишком поздно, чтобы быть полезным для ОП, но я склонен думать, что сложность здесь заключается скорее в запахе кода.Если мы рассмотрим принцип единой ответственности на уровне метода, а не просто класса, то, похоже, этот метод его нарушает.Он делает 2 вещи: он анализирует data, а затем выполняет дальнейшую обработку.Таким образом, правильное решение состоит в том, чтобы устранить одну ответственность, а именно, разбор.Вы можете сделать это, изменив аргумент на экземпляр AnotherClass.Затем становится ясно, что вы должны делать:

public void MyMethod(AnotherClass data)
{
    if (data == null)
        throw new ArgumentNullException("data is null.");

    if (data.SomeProperty < 0)
        throw new ArgumentException("data.SomeProperty must not be negative.");

    ...
}

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

Answer 8

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

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

• Связанные с безопасностью (уполномочен ли вызывающий абонент выполнять вызов?)
• Связаны с контекстом (находится ли объект справасостояние для совершения вызова?)
• Связанный с вводом (имеет ли вызывающий объект допустимые аргументы?)

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

• SecurityException
• InvalidOperationException
• ArgumentException

Эти исключения сообщают вызывающей стороне, что «по их вине» что-то пошло не так и вызов не может быть завершен правильно.Однако, если все предварительные условия выполняются в соответствии с формальной спецификацией метода, и метод обнаруживает, что он не может соответствовать указанным постусловиям, он должен выбросить Exception типа, который четко сообщает , что пошло не так.Как правило, вы хотите определить свой собственный тип исключения для этих ситуаций или повторно использовать существующий тип исключения, если это не один из типов, зарезервированных для сбоев предусловий.

И, наконец, ошибкиExceptions, которые выдает CLR, которые могут возникать практически в любом месте, в любое время и которые практически невозможно предсказать.Они никогда не будут явным образом частью ваших методов и, как таковые, никогда не должны выбрасываться (ни обрабатываться специально) пользовательским кодом.В этом смысле они сопоставляют почти однозначное с фатальными исключениями Липперта.

Так что вы должны сделать, на мой взгляд, в случае, представленном Слаумой?

Ну, как выМожно себе представить, это полностью зависит от контрактов MyMethod(data), GetObject(data) и objectOfAnotherClass.SomeProperty.Что говорит API GetObject(data) о том, в каких ситуациях он вернет null (или выбросит свои собственные исключения)?Каково точное значение и спецификация SomeProperty?

Предположим, что GetObject(data) - это метод, который возвращает объект, извлеченный из базы данных, а data - это идентификатор объекта.Если в контракте GetObject(data) указано, что он вернет null, если не существует объекта с идентификатором data, то ваш дизайн должен учитывать это в своем собственном контракте.Возможно, вы захотите заставить вызывающего абонента, если это разумно, всегда указывать значение для data, чтобы GetObject(data) не возвращало null, а если это так, вы можете бросить ArgumentException (или производную), чтобы указатьошибка на стороне вызывающей стороны.

С другой стороны, если GetObject(data) указывает, что возвращает ноль, только когда у вызывающей стороны недостаточно прав для извлечения объекта, вы можете бросить SecurityException (или производную) длясообщить о проблеме вызывающему абоненту MyMethod(data).

Наконец, если GetObject(data) обещает, что он "никогда" не вернет null, и это все равно происходит, вы можете позволить своему коду аварийно завершить работу с NullReferenceException (потому что справедливо полагая, что это никогда не будет null) или обрабатывать ситуацию, особенно когда вы имеете дело с чувствительным кодом, бросая свой собственный тип исключения (поскольку постусловие, с точки зрения вызывающей стороны MyMethod, не удалось).

Второй случай немного сложнее, но к нему можно подходить почти так же.Предположим, что objectOfAnotherClass представляет строку или сущность, которые были извлечены из базы данных по идентификатору data, а MyMethod(data) четко указывает, что data должен указывать идентификатор объекта, где objectOfAnotherClass.SomeProperty >= 0, тогда в этом случае выдаетсяArgumentException (или производная) будет частью вашего проекта.

Опять же, когда MyMethod(data) работает в контексте, где вызывающий может предположить, что действительный идентификатор никогда не вернет объект, такой что objectOfAnotherClass.SomeProperty < 0, (или лучше: даже не подозревает, что такой объект существует), тогда такое появление действительно неожиданно на сайте вызывающего абонента, и MyMethod(data) не должен даже явно проверять случай (опять же, если предположить, что это не произойдет), или, если будет болеенадежный код, сгенерируйте определенное, пользовательское исключение, указывающее на проблему.

В заключение: я думаю, что определение типа Exception для выброса зависит исключительно от формальных контрактов, которые каждый метод имеет со своими вызывающими и вызываемыми.Если вызывающий не удовлетворяет предварительному условию, метод должен всегда отвечать вызывающему, бросая SecurityException, InvalidOperationException или ArgumentException.Если сам метод не соответствует его собственным постусловиям, или может произойти сбой исключений, выданных CLR или другими компонентами, или выдать свое собственное более конкретное исключение, указывающее на проблему.

Answer 9

Это действительно зависит от того, как вы планируете обрабатывать исключения в вашем приложении.Пользовательские исключения хороши в ситуациях try / catch, но try / catch также дороги.Если вы не планируете отлавливать и обрабатывать свое собственное исключение, то: выведите новое исключение («Индекс вне диапазона: SomeProperty не должен быть отрицательным.»);так же полезен, как и пользовательское исключение.

public class InvalidStateException : ApplicationException
{
   ...
}

В вашем коде:

// test for null
if(objectOfAnotherClass == null) throw new NullReferenceException("Object cannot be null");

// test for valid state
if(objectOfAnotherClass.SomeProperty < 0) throw new InvalidStateException("Object is in an invalid state");