Проверка параметров конструктора в C # - Лучшие практики

34

Какова лучшая практика для проверки параметров конструктора?

Предположим, что немного C #:

public class MyClass
{
    public MyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            throw new ArgumentException("Text cannot be empty");

        // continue with normal construction
    }
}

Было бы приемлемо бросить исключение?

Альтернативой, с которой я столкнулся, была предварительная проверка перед созданием экземпляра:

public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
        {
            MessageBox.Show("Text cannot be empty");
            return null;
        }
        else
        {
            return new MyClass(text);
        }
    }
}
MPelletier
источник
10
"допустимо ли бросить исключение?" Какая альтернатива?
S.Lott
10
@ S.Lott: ничего не делать. Установите значение по умолчанию. Распечатайте сообщение в консоль / файл журнала. Звонить в звонок. Мигать свет.
FrustratedWithFormsDesigner
3
@FrustratedWithFormsDesigner: "Ничего не делать?" Как будет выполнено ограничение «текст не пустой»? «Установить значение по умолчанию»? Как будет использоваться значение текстового аргумента? Другие идеи хороши, но эти две приведут к объекту в состоянии, которое нарушает ограничения на этот класс.
С.Лотт
1
@ S.Lott Из-за боязни повториться, я был открыт к тому, что есть какой-то другой подход, которого я не знал. Я полагаю, что я не все знаю, это я знаю наверняка.
MPelletier
3
@MPelletier, проверка параметров заранее приведет к нарушению DRY. (Не повторяйте себя). Поскольку вам необходимо скопировать эту предварительную проверку в любой код, который пытается создать экземпляр этого класса.
CaffGeek

Ответы:

26

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

if (string.IsNullOrEmpty(text))
    throw new ArgumentException("message", "text");

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

if (string.IsNullOrWhiteSpace(text))
    throw new ArgumentException("message", "text");
ChaosPandion
источник
Я не уверен, что его «конкретный случай» достаточно конкретен, чтобы давать такие конкретные советы. Мы понятия не имеем, имеет ли смысл в этом контексте генерировать исключение или просто позволить объекту существовать в любом случае.
FrustratedWithFormsDesigner
@FrustratedWithFormsDesigner - я полагаю, вы меня за это проголосовали? Конечно, вы правы в том, что я экстраполирую, но ясно, что этот теоретический объект должен быть недействительным, если входной текст пуст. Вы действительно хотели бы вызвать, Initчтобы получить какой-то код ошибки, когда исключение будет работать так же хорошо и заставлять ваш объект всегда поддерживать действительное состояние?
ChaosPandion
2
Я склонен разбивать этот первый случай на два отдельных исключения: одно проверяет на ничтожность и выбрасывает, ArgumentNullExceptionа другое проверяет пустое и бросает ArgumentException. Позволяет вызывающему быть более разборчивым, если он хочет в обработке исключений.
Джесси С. Slicer
1
@Jesse - я использовал эту технику, но до сих пор не могу понять, насколько она полезна.
ChaosPandion
3
+1 за неизменяемые объекты! Я также использую их везде, где это возможно (лично я нахожу почти всегда).
22

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

В C ++ выбрасывать исключение из конструктора - плохая идея, потому что он оставляет вам выделенную память, содержащую неинициализированный объект, на который у вас нет ссылок (т.е. это классическая утечка памяти). Вероятно, отсюда и возникает стигма - куча разработчиков старой школы на C ++ наполовину отучились от обучения C # и просто применили к нему то, что знали из C ++. Напротив, в Objective-C Apple отделяла этап выделения от этапа инициализации, поэтому конструкторы на этом языке могут генерировать исключения.

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

Муравей
источник
Ты так взъерошишь некоторые перья своим комментарием C ++. :)
ChaosPandion
5
Эхх, C ++ является отличным языком, но один из моих любимых мозолей есть люди , которые изучают один язык хорошо , а затем написать как то все время . C # не C ++.
Муравей
10
Все еще может быть опасно генерировать исключения из ctor в C #, потому что ctor мог бы выделить неуправляемые ресурсы, которые затем никогда не будут утилизированы. И финализатор должен быть написан для защиты от сценария, когда не полностью построенный объект завершен. Но эти сценарии относительно редки. В следующей статье, посвященной центру разработки C #, будут рассмотрены эти сценарии и способы их защиты, поэтому следите за подробностями в этом месте.
Эрик Липперт
6
а? бросать исключение из CTOR это единственное , что вы можете сделать в C ++ , если вы не можете построить объект, и нет никаких причин , это имеет вызвать утечку памяти (хотя , очевидно , его до).
JK.
@jk: Хм, ты прав! Хотя кажется, что альтернатива - помечать плохие объекты как «зомби», что, очевидно, делает STL вместо исключения: yosefk.com/c++fqa/exceptions.html#fqa-17.2 Решение Objective-C гораздо более аккуратно.
Муравей
12

Бросить исключение IFF, класс не может быть приведен в согласованное состояние в отношении его семантического использования. В противном случае нет. НИКОГДА не позволяйте объекту существовать в несогласованном состоянии. Это включает в себя не предоставление полных конструкторов (например, наличие пустого конструктора + initialize () до того, как ваш объект фактически полностью построен) ... ПРОСТО СКАЗАТЬ НЕТ!

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

Я должен отметить, что под «конструктором» я подразумеваю то, что клиент вызывает для создания объекта. Это также может быть что-то иное, чем реальная конструкция, которая называется «Конструктор». Например, что-то подобное в C ++ не будет нарушать принцип IMNSHO:

struct funky_object
{
  ...
private:
  funky_object();
  bool initialize(std::string);

  friend boost::optional<funky_object> build_funky(std::string);
};
boost::optional<funky_object> build_funky(std::string str)
{
  funky_object fo;
  if (fo.initialize(str)) return fo;
  return boost::optional<funky_object>();
}

Поскольку единственный способ создать это funky_object- вызвать build_funkyпринцип, согласно которому недопустимый недопустимый объект остается неизменным, даже если фактический «Конструктор» не завершает работу.

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

Эдвард Стрендж
источник
9

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

public class MyClass
{
    private MyClass(string text)
    {
        //normal construction
    }

    public static MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            return null;
        else
            return new MyClass(text);
    }
}
public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        var cls = MyClass.MakeMyClass(text);
        if(cls == null)
             //show messagebox or throw exception
        return cls;
    }
}

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

Донн реласьон
источник
1
Я не вижу преимущества. Почему для проверки результата фабрики на null предпочтительнее, чем где-нибудь использовать try-catch, который может перехватить исключение из конструктора? Кажется, ваш подход «игнорировать исключения, вернитесь к явной проверке возвращаемого значения из методов на наличие ошибок». Мы давно признали, что исключения, как правило, превосходят необходимость явно проверять возвращаемое значение каждого метода на наличие ошибок, поэтому ваш совет кажется подозрительным. Я хотел бы увидеть какое-то обоснование того, почему вы считаете, что общее правило здесь не применимо.
DW
мы никогда не признавали, что исключения превосходят коды возврата, они могут быть огромным ударом по производительности, если их выдают слишком часто. Однако создание исключения из конструктора приведет к утечке памяти, даже в .NET, если вы выделили что-то неуправляемое. Вы должны сделать большую работу в своем финализаторе, чтобы восполнить это дело. В любом случае, фабрика - это хорошая идея, и TBH должна выдавать исключение, а не возвращать ноль. Предполагая, что вы создаете только несколько «плохих» классов, тогда сделать фабричный бросок предпочтительнее.
gbjbaanb
2
  • Конструктор не должен иметь побочных эффектов.
    • Все, кроме инициализации частного поля, следует рассматривать как побочный эффект.
    • Конструктор с побочными эффектами нарушает принцип единой ответственности (SRP) и противоречит духу объектно-ориентированного программирования (ООП).
  • Конструктор должен быть легким и никогда не должен выходить из строя.
    • Например, я всегда дрожу, когда вижу блок try-catch внутри конструктора. Конструктор не должен выбрасывать исключения или регистрировать ошибки.

Можно разумно поставить под сомнение эти рекомендации и сказать: «Но я не следую этим правилам, и мой код работает нормально!» На это я бы ответил: «Ну, это может быть правдой, пока это не так».

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

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

Так что это зависит от того, насколько важен текст параметра для метода - имеет ли объект смысл с нулевым или пустым значением?

Стивен Стрига
источник
2
Я думаю, что вопрос подразумевает, что класс на самом деле требует, чтобы строка была не пустой. Это не так в вашем примере StringBuilder.
Серхио Акоста,
Тогда ответ должен быть да - допустимо бросить исключение. Чтобы избежать необходимости пытаться / отловить эту ошибку, вы можете использовать фабричный шаблон для обработки создания объекта, который будет включать в себя случай пустой строки.
Стивен Стрига
0

Я предпочитаю установить значение по умолчанию, но я знаю, что в Java есть библиотека «Apache Commons», которая делает что-то подобное, и я думаю, что это тоже довольно хорошая идея. Я не вижу проблемы с выдачей исключения, если недопустимое значение приведет объект в непригодное для использования состояние; строка не хороший пример, но что, если она была для DI бедного человека? Вы не можете работать, если nullвместо ICustomerRepositoryинтерфейса , скажем, передано значение . Я бы сказал, что в такой ситуации исключение является правильным способом обработки вещей.

Уэйн Молина
источник
-1

Когда я работал в C ++, я не использовал генерацию исключений в конструкторе из-за проблемы утечки памяти, к которой это может привести, я научился этому нелегко. Любой, кто работает с c ++, знает, насколько сложной и проблемной может быть утечка памяти.

Но если вы находитесь в C # / Java, то у вас нет этой проблемы, потому что сборщик мусора будет собирать память. Если вы используете C #, я думаю, что лучше использовать свойство для удобного и согласованного способа убедиться, что ограничения данных гарантированы.

public class MyClass
{
    private string _text;
    public string Text 
    {
        get
        {
            return _text;
        } 
        set
        {
            if (string.IsNullOrWhiteSpace(value))
                throw new ArgumentException("Text cannot be empty");
            _text = value;
        } 
    }

    public MyClass(string text)
    {
        Text = text;
        // continue with normal construction
    }
}
ajitdh
источник
-3

Бросок исключения станет настоящей болью для пользователей вашего класса. Если проверка является проблемой, то я обычно делаю создание объекта в два этапа.

1 - Создать экземпляр

2 - Вызвать метод Initialize с возвращаемым результатом.

ИЛИ

Вызовите метод / функцию, которая создаст для вас класс, который может выполнить проверку.

ИЛИ

У меня есть флаг isValid, который мне не особенно нравится, но некоторые люди делают.

Замочить
источник
3
@ Данк, это просто неправильно.
CaffGeek
4
@ Чад, это твое мнение, но мое мнение не так. Это просто отличается от вашего. Я считаю, что код try-catch гораздо труднее читать, и я видел гораздо больше ошибок, возникающих, когда люди используют try-catch для обработки тривиальных ошибок, чем более традиционные методы. Это был мой опыт, и это не так. Что есть, то есть.
Данк
5
ТОЧКА в том, что после того, как я вызываю конструктор, если он не создается из-за того, что ввод неверен, это ИСКЛЮЧЕНИЕ. Данные приложения теперь находятся в несогласованном / недопустимом состоянии, если я просто создаю объект и устанавливаю флаг, говоря isValid = false. Необходимость проверки после того, как класс создан, ужасен, очень подвержен ошибкам дизайн. И, как вы сказали, есть конструктор, затем вызовите initialize ... Что если я не вызову initialize? Что тогда? А что, если Initialize получит неверные данные, могу ли я выдать исключение сейчас? Ваш класс не должен быть сложным в использовании.
CaffGeek
5
@ Данк, если вам трудно использовать исключения, вы их неправильно используете. Проще говоря, плохой вклад был предоставлен конструктору. Это исключение. Приложение не должно продолжать работать, пока оно не исправлено. Период. Если это произойдет, вы получите худшую проблему, плохие данные. Если вы не знаете, как обрабатывать исключение, вы этого не сделаете, вы позволяете ему пузыриться в стеке вызовов, пока он не будет обработан, даже если это означает просто запись в журнал и остановку выполнения. Проще говоря, вам не нужно обрабатывать исключения или проверять их. Они просто работают.
CaffGeek
3
Извините, это слишком много анти-паттерна для рассмотрения.
MPelletier