Каковы хорошие способы сбалансировать информативные исключения и чистый код?

11

С нашим общедоступным SDK мы стремимся давать очень информативные сообщения о том, почему возникает исключение. Например:

if (interfaceInstance == null)
{
     string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    throw new InvalidOperationException(errMsg);
}

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

Коллега начал рефакторинг некоторых исключений, бросая что-то вроде этого:

if (interfaceInstance == null)
    throw EmptyConstructor();

...

private Exception EmptyConstructor()
{
    string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    return new InvalidOperationException(errMsg);
}

Что облегчает понимание логики кода, но добавляет множество дополнительных методов для обработки ошибок.

Каковы другие способы избежать проблемы "логики беспорядка длинных сообщений об исключениях"? Я в первую очередь спрашиваю об идиоматическом C # / .NET, но как другие языки управляют им, также полезно.

[Редактировать]

Было бы неплохо иметь плюсы и минусы каждого подхода.

FriendlyGuy
источник
4
ИМХО, решение вашего коллеги очень хорошее, и я думаю, что если вы получите действительно много дополнительных методов такого рода, вы можете использовать хотя бы некоторые из них. Хорошо иметь много небольших методов, когда ваши методы имеют правильные имена, если они создают простые для понимания строительные блоки вашей программы - здесь, похоже, так и есть.
Док Браун
@DocBrown - Да, мне нравится идея (добавлена ​​в качестве ответа ниже, вместе с плюсами / минусами), но как для intellisense, так и для потенциального числа методов, она также начинает выглядеть как беспорядок.
FriendlyGuy
1
Мысль: не сделать сообщения о носителе всех деталей исключения. Комбинация использования Exception.Dataсвойства, «выборочной» перехвата исключений, перехвата кода и добавления собственного контекста вместе с перехваченным стеком вызовов - все это вносит информацию, которая должна позволять гораздо меньше многословных сообщений. Наконец, System.Reflection.MethodBaseвыглядит многообещающим для предоставления деталей для перехода к вашему методу "создания исключений".
Радар Боб
@radarbob ты предполагаешь, что, возможно, исключения слишком многословны? Возможно, мы делаем это слишком похоже на регистрацию.
FriendlyGuy
1
@MackleChan, я читал, что парадигма здесь заключается в том, чтобы поместить информацию в сообщение, которое пытается точно сказать, что произошло, обязательно грамматически правильно, и притворство ИИ: «... через пустой конструктор сработало, но ...» В самом деле? Мой внутренний криминалист рассматривает это как эволюционное следствие распространенной ошибки повторных бросков с потерей следа стека и незнанием Exception.Data. Акцент должен быть захват телеметрии. Рефакторинг здесь хорошо, но он не решает проблему.
Радар Боб

Ответы:

10

Почему бы не иметь специализированные классы исключений?

if (interfaceInstance == null)
{
    throw new ThisParticularEmptyConstructorException(<maybe a couple parameters>);
}

Это переносит форматирование и детали к самому исключению и оставляет основной класс незагроможденным.

ptyx
источник
1
Плюсы: очень чистый и организованный, обеспечивает значимые имена для каждого исключения. Минусы: потенциально много дополнительных исключений классов.
FriendlyGuy
Если это имеет значение, вы можете иметь ограниченное количество классов исключений, передать класс, где исключение возникло как параметр, и иметь гигантский переключатель внутри более общих классов исключений. Но это имеет слабый запах - не уверен, что я пойду туда.
ptyx
это пахнет очень плохо (тесно связанные исключения с классами). Я думаю, было бы сложно сбалансировать настраиваемые сообщения об исключениях и количество исключений.
FriendlyGuy
7

Кажется, что Microsoft (просматривая исходный код .NET) иногда использует строки ресурсов / среды. Например, ParseDecimal:

throw new OverflowException(Environment.GetResourceString("Overflow_Decimal"));

Плюсы:

  • Централизация сообщений об исключениях, позволяющая повторное использование
  • Хранение сообщения об исключении (возможно, которое не имеет значения для кода) подальше от логики методов
  • Тип создаваемого исключения ясен
  • Сообщения могут быть локализованы

Минусы:

  • Если одно сообщение об исключении изменилось, все они изменились
  • Сообщение об исключении не так легко доступно для кода, генерирующего исключение.
  • Сообщение является статическим и не содержит информации о том, какие значения неверны. Если вы хотите отформатировать его, это больше беспорядка в коде.
FriendlyGuy
источник
2
Вы упустили огромную выгоду: локализация текста исключения
17 из 26
@ 17of26 - Хорошо, добавил.
FriendlyGuy
Я проголосовал за ваш ответ, но сообщения об ошибках, сделанные таким образом, являются «статическими»; Вы не можете добавлять к ним модификаторы, как это делает OP в своем коде. Таким образом, вы фактически упустили некоторые из его функций.
Роберт Харви
@RobertHarvey - я добавил это как мошенничество. Это объясняет, почему встроенные исключения никогда не дают вам локальную информацию. Кроме того, к вашему сведению, я OP (я знал об этом решении, но я хотел знать, есть ли у других лучшие решения).
FriendlyGuy
6
@ 17of26 как разработчик, я ненавижу локализованные исключения со страстью. Я должен вывести их из строя каждый раз, прежде чем смогу, например. Google для решений.
Конрад Моравский
2

Для общедоступного сценария SDK я настоятельно рекомендую использовать Microsoft Code Contracts, поскольку они предоставляют информативные ошибки, статические проверки, а также вы можете создавать документацию для добавления в документы XML и файлы справки, созданные в Sandcastle . Поддерживается во всех платных версиях Visual Studio.

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

Полная документация по кодовым контрактам находится здесь .

Мир джеймса
источник
2

Техника, которую я использую, состоит в том, чтобы объединить и передать проверку и использование функции полезности.

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

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

Конечно, есть способы сделать это - строго типизированный язык, дизайн «недопустимые объекты запрещены в любое время», проектирование по контракту и т. Д.

Пример:

internal static class ValidationUtil
{
    internal static void ThrowIfRectNullOrInvalid(int imageWidth, int imageHeight, Rect rect)
    {
        if (rect == null)
        {
            throw new ArgumentNullException("rect");
        }
        if (rect.Right > imageWidth || rect.Bottom > imageHeight || MoonPhase.Now == MoonPhase.Invisible)
        {
            throw new ArgumentException(
                message: "This is uselessly informative",
                paramName: "rect");
        }
    }
}

public class Thing
{
    public void DoSomething(Rect rect)
    {
        ValidationUtil.ThrowIfRectNullOrInvalid(_imageWidth, _imageHeight, rect);
        // rest of your code
    }
}
rwong
источник
1

[примечание] Я скопировал это из вопроса в ответ, если есть комментарии по этому поводу.

Переместите каждое исключение в метод класса, принимая любые аргументы, которые требуют форматирования.

private Exception EmptyConstructor()
{
    string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    return new InvalidOperationException(errMsg);
}

Вложите все методы исключения в регион и поместите их в конец класса.

Плюсы:

  • Хранит сообщение вне основной логики метода
  • Позволяет добавлять логическую информацию к каждому сообщению (вы можете передать аргументы методу)

Минусы:

  • Метод беспорядка. Потенциально у вас может быть много методов, которые просто возвращают исключения и на самом деле не связаны с бизнес-логикой.
  • Невозможно повторно использовать сообщения в других классах
FriendlyGuy
источник
Я думаю, что минусы, на которые вы ссылались, значительно перевешивают преимущества.
Неонтапир
@neontapir все минусы легко решаются. Вспомогательные методы должны быть заданы как IntentionRevealingNames и сгруппированы в некоторые #region #endregion(что по умолчанию скрывает их от IDE), и, если они применимы для разных классов, поместите их в internal static ValidationUtilityкласс. Кстати, никогда не жалуйтесь на длинные имена перед программистом C #.
Rwong
Я бы пожаловался на регионы, хотя. На мой взгляд, если вы захотите прибегнуть к регионам, у класса, вероятно, было слишком много обязанностей.
Неонтапир
0

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

public static I CastOrThrow<I,T>(T t, string source)
{
    if (t is I)
        return (I)t;

    string errMsg = string.Format(
          "Failed to complete {0}, because type: {1} could not be cast to type {2}.",
          source,
          typeof(T),
          typeof(I)
        );

    throw new InvalidOperationException(errMsg);
}


/// and then:

var interfaceInstance = SdkHelper.CastTo<IParameter>(passedObject, "Action constructor");

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

Если вы работаете в .net 4.5, есть способы заставить компилятор вставить имя текущего метода / файла в качестве параметра метода (см. CallerMemberAttibute ). Но для SDK вы, вероятно, не можете требовать от своих клиентов перехода на 4.5.

JDV-Ян де Ваан
источник
Больше о исключениях, генерируемых в целом (и об управлении информацией в исключении, а не о том, насколько сильно она загромождает код), а не об этом конкретном примере приведения.
FriendlyGuy
0

Что нам нравится делать для ошибок бизнес-логики (не обязательно ошибок аргументов и т. Д.), Так это иметь единственное перечисление, которое определяет все потенциальные типы ошибок:

/// <summary>
/// This enum is used to identify each business rule uniquely.
/// </summary>
public enum BusinessRuleId {

    /// <summary>
    /// Indicates that a valid body weight value of a patient is missing for dose calculation.
    /// </summary>
    [Display(Name = @"DoseCalculation_PatientBodyWeightMissing")]
    PatientBodyWeightMissingForDoseCalculation = 1,

    /// <summary>
    /// Indicates that a valid body height value of a patient is missing for dose calculation.
    /// </summary>
    [Display(Name = @"DoseCalculation_PatientBodyHeightMissing")]
    PatientBodyHeightMissingForDoseCalculation = 2,

    // ...
}

Эти [Display(Name = "...")]атрибуты определяют ключ в ресурсе файлов , которые будут использоваться для перевода сообщений об ошибках.

Кроме того, этот файл можно использовать в качестве отправной точки для поиска всех случаев, когда в вашем коде генерируется ошибка определенного типа.

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

Затем мы используем пользовательский тип исключения для транспортировки нарушенных правил:

[Serializable]
public class BusinessLogicException : Exception {

    /// <summary>
    /// The Business Rule that was violated.
    /// </summary>
    public BusinessRuleId ViolatedBusinessRule { get; set; }

    /// <summary>
    /// Optional: additional parameters to be used to during generation of the error message.
    /// </summary>
    public string[] MessageParameters { get; set; }

    /// <summary>
    /// This exception indicates that a Business Rule has been violated. 
    /// </summary>
    public BusinessLogicException(BusinessRuleId violatedBusinessRule, params string[] messageParameters) {
        ViolatedBusinessRule = violatedBusinessRule;
        MessageParameters = messageParameters;
    }
}

Вызовы сервисной службы заключены в общий код обработки ошибок, который переводит нарушенное правило Busines в удобочитаемое сообщение об ошибке:

public object TryExecuteServiceAction(Action a) {
    try {
        return a();
    }
    catch (BusinessLogicException bex) {
        _logger.Error(GenerateErrorMessage(bex));
    }
}

public string GenerateErrorMessage(BusinessLogicException bex) {
    var translatedError = bex.ViolatedBusinessRule.ToTranslatedString();
    if (bex.MessageParameters != null) {
        translatedError = string.Format(translatedError, bex.MessageParameters);
    }
    return translatedError;
}

Вот ToTranslatedString()метод расширения, enumкоторый может считывать ключи ресурсов из [Display]атрибутов и использовать их ResourceManagerдля перевода. Значение для соответствующего ключа ресурса может содержать заполнители для string.Format, которые соответствуют предоставленным MessageParameters. Пример записи в файле resx:

<data name="DoseCalculation_PatientBodyWeightMissing" xml:space="preserve">
    <value>The dose can not be calculated because the body weight observation for patient {0} is missing or not up to date.</value>
    <comment>{0} ... Patient name</comment>
</data>

Пример использования:

throw new BusinessLogicException(BusinessRuleId.PatientBodyWeightMissingForDoseCalculation, patient.Name);

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

Георг Патшайдер
источник
0

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

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

Мартин К
источник