2013-07-18 8 views
1

У меня есть класс, который выглядит следующим образом:Где следует исключать исключения?

public class StackOverflowQuestion { 

    private string _question; 

    public string Question { 
     get { return _question; } 
     set { _question = value; } 
    } 

    public StackOverflowQuestion(string question) { 
     _question = question; 
    } 

    public override string ToString() { 
     return _question; 
    } 
} 

Теперь, «вопрос» значение не может быть нулевым или пустым, и пользователь должен быть уведомлен через ArgumentNullException - но там, где оно должно было бросили? Согласно принципу «неудачный» -> везде.

public class StackOverflowQuestion { 

    private string _question; 

    public string Question { 
     get { return _question; } 
     set { 
      if(!String.IsNullOrEmpty(value)) 
       _question = value 
      else throw new ArgumentNullException("value"); 
     } 
    } 

    public StackOverflowQuestion(string question) { 
     if(!String.IsNullOrEmpty(question)) 
      _question = question; 
     else throw new ArgumentNullException("question"); 
    } 

    public override string ToString() { 
     if(!String.IsNullOrEmpty(_question)) return _question; 
     else throw new ArgumentNullException("_question"); 
    } 
} 

Теперь это, очевидно, нелепо и чрезвычайно повторяемо. Но это кажется правильным: если значение установлено через .ctor, он выходит из строя сразу после короткой проверки. Когда он устанавливается через свойство, он терпит неудачу сразу после короткой проверки .. но кто ожидает исключений из сеттера? И когда я выводя строку, я ожидаю, что строка, а не исключение для чего-то, что должно было произойти давно, но опять же: если это не так, оно должно потерпеть неудачу, даже если «скоро» довольно поздно.

Итак, где должна выполняться обработка исключительных ситуаций? Я прошу о «лучшей практике», или это вкус?

+1

ToString не должен бросить исключение –

+0

Вы можете уточнить, почему нет? – Atrotygma

+1

@Atrotygma: Потому что он никогда не может попасть в это состояние ... –

ответ

3

Поскольку _question является частным, есть нет необходимости проверять, является ли оно нулевым в ToString() (если только вы не просто проверяете свой собственный код).

Вы можете избежать проверки конструктора, указав конструктор, используя свойство setter. Таким образом, я бы рекомендовал:

public class StackOverflowQuestion { 

    private string _question; 

    public string Question { 
     get { return _question; } 
     set { 
      if(string.IsNullOrEmpty(value)) 
       // to make this more transparent when thrown through the constructor, it might 
       // be preferable to throw a real error message like "Question: cannot be null or empty" 
       throw new ArgumentException("value"); 
      this._question = value; 
     } 
    } 

    public StackOverflowQuestion(string question) { 
     this.Question = question; 
    } 

    public override string ToString() { 
     return this.Question; 
    } 
} 

Несколько вещей отметить: 1. Вы должны бросить ArgumentException, а не ArgumentNullException для пустых строк (если вы хотите, вы можете сделать 2 проверки и до сих пор бросают ArgumentNullException для нулям). 2. Хотя подход использует меньше кода, одним из недостатков является то, что пользователи сообщений об ошибках немного хуже, чем когда они передают значение null конструктору, поскольку сбой происходит на 2 уровня, а не один.

2

Вам нужно только проверить его раз - в одном месте, где вы установите переменную, изменив конструктор использовать свойство:

public class StackOverflowQuestion 
{ 
    private string _question; 

    public string Question 
    { 
     get { return _question; } 
     set 
     { 
      if (String.IsNullOrEmpty(value)) 
      { 
       throw new ArgumentException("Question cannot be null or empty", 
              "value"); 
      } 
      _question = value; 
     } 
    } 

    public StackOverflowQuestion(string question) 
    { 
     Question = question; 
    } 

    public override string ToString() 
    { 
     return Question; 
    } 
} 

Один недостаток в том, что «плохой параметр «имя будет value, а не question, когда в конструкторе будет null, но я думаю, что это цена, которую стоит заплатить. Альтернативой является только, чтобы использовать это сообщение, а не указывать имя параметра.

Вы может хочет отделить null от empty, так что вы можете бросить ArgumentNullException когда это нуль - но вы не должны бросать ArgumentNullException, когда это просто пустые.

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

Вы должны также рассмотреть, можно ли сделать класс неизменны, и в этот момент вам нужно только проверить в конструкторе, как не было бы сеттер ...

+0

Это то, о чем я думал слишком сначала, но в этом случае исключения отбрасываются на свойство - это дает трассировку стека, которую я не ожидаю, потому что я дал переменная в ctor, а не свойство. Или я что-то не так понял? – Atrotygma

+0

Я думаю, что соглашение .NET Framework должно использовать имя свойства, а не «значение». Это, безусловно, передает немного больше информации. –

+0

@Atrotygma: Трассировка стека по-прежнему включает вызов, который вы * сделали * make. Я не думаю, что вы должны ожидать, что трассировка стека всегда будет * напрямую * в точке, которую вы вызывали. –

1

я предпочел бы сделать это неизменное:

public class StackOverflowQuestion 
    { 
     public string Question 
     { 
      get; private set; 
     } 

     public StackOverflowQuestion(string question) 
     { 
      if (String.IsNullOrEmpty(question))     
       throw new ArgumentNullException("question"); 

      Question = question; 
     } 

     public override string ToString() 
     { 
      return Question; 
     } 
    } 
Смежные вопросы