Ответ 1
Я всегда делаю исключение в этом случае. Подумайте, используя InvalidEnumArgumentException
, который дает более подробную информацию в этой ситуации.
Скажем, у нас есть функция, которая изменяет пароль для пользователя в системе в приложении MVC.
public JsonResult ChangePassword
(string username, string currentPassword, string newPassword)
{
switch (this.membershipService.ValidateLogin(username, currentPassword))
{
case UserValidationResult.BasUsername:
case UserValidationResult.BadPassword:
// abort: return JsonResult with localized error message
// for invalid username/pass combo.
case UserValidationResult.TrialExpired
// abort: return JsonResult with localized error message
// that user cannot login because their trial period has expired
case UserValidationResult.Success:
break;
}
// NOW change password now that user is validated
}
membershipService.ValidateLogin()
возвращает a UserValidationResult
перечисление, которое определяется как:
enum UserValidationResult
{
BadUsername,
BadPassword,
TrialExpired,
Success
}
Будучи защитным программистом, я бы изменил вышеописанный метод ChangePassword()
, чтобы выбросить исключение, если есть недопустимое значение UserValidationResult
от ValidateLogin()
:
public JsonResult ChangePassword
(string username, string currentPassword, string newPassword)
{
switch (this.membershipService.ValidateLogin(username, currentPassword))
{
case UserValidationResult.BasUsername:
case UserValidationResult.BadPassword:
// abort: return JsonResult with localized error message
// for invalid username/pass combo.
case UserValidationResult.TrialExpired
// abort: return JsonResult with localized error message
// that user cannot login because their trial period has expired
case UserValidationResult.Success:
break;
default:
throw new NotImplementedException
("Unrecognized UserValidationResult value.");
// or NotSupportedException()
break;
}
// Change password now that user is validated
}
Я всегда считал образец, похожий на последний фрагмент, выше лучшей практики. Например, что, если один разработчик получит требование о том, что теперь, когда пользователи пытаются войти в систему, если по этому-то-делому причинам они должны сначала обратиться в бизнес? Таким образом, определение UserValidationResult
обновляется следующим образом:
enum UserValidationResult
{
BadUsername,
BadPassword,
TrialExpired,
ContactUs,
Success
}
Разработчик меняет тело метода ValidateLogin()
, чтобы вернуть новое значение enum (UserValidationResult.ContactUs
), если это применимо, но забывает обновить ChangePassword()
. Без исключения в коммутаторе пользователю по-прежнему разрешено изменять свой пароль, когда их попытка входа в систему не должна быть проверена в первую очередь!
Это только я, или это default: throw new Exception()
хорошая идея? Я видел это несколько лет назад, и всегда (после того, как он это сделал) предположил, что это лучшая практика.
Я всегда делаю исключение в этом случае. Подумайте, используя InvalidEnumArgumentException
, который дает более подробную информацию в этой ситуации.
С тем, что у вас есть, это хорошо, хотя инструкция break после нее никогда не будет удалена, потому что выполнение этого потока прекратится, когда исключение будет отправлено и необработано.
Я использовал эту практику раньше для конкретных случаев, когда перечисление живет "далеко" от нее, но в тех случаях, когда перечисление действительно близко и посвящено определенной функции, это кажется немного немного.
По всей видимости, я подозреваю, что отказ от отладки, вероятно, более уместен.
http://msdn.microsoft.com/en-us/library/6z60kt1f.aspx
Обратите внимание на второй пример кода...
Мне всегда нравится делать то, что у вас есть, хотя обычно я вывожу ArgumentException
, если это аргумент, который был передан, но я вроде как NotImplementedException
лучше, так как ошибка, скорее всего, а не вызывающий должен изменить аргумент на поддерживаемый.
Я никогда не добавляю разрыв после инструкции throw, содержащейся в инструкции switch. Не в последнюю очередь это опасное предупреждение о недостижимом коде. Так что да, это хорошая идея.
Я думаю, что такие подходы могут быть очень полезными (например, см. Ошибочно пустые коды кода). Это даже лучше, если вы можете заставить этот бросок по умолчанию быть скомпилированным в выпущенном коде, например, assert. Таким образом, у вас есть дополнительная защита во время разработки и тестирования, но при освобождении не начисляйте лишние накладные расходы.
Вы заявляете, что находитесь в очень оборонительной ситуации, и я могу сказать, что это за борт. Разве другой разработчик не будет проверять свой код? Разумеется, когда они проводят простейшие тесты, они поймут, что пользователь все равно может войти в систему, поэтому они поймут, что им нужно исправить. То, что вы делаете, не ужасно или неправильно, но если вы тратите много времени на это, это может быть слишком много.
Для веб-приложения я предпочел бы по умолчанию генерировать результат с сообщением об ошибке, которое просит пользователя обратиться к администратору и регистрировать ошибку, а не вызывать исключение, которое может привести к чему-то менее значимому для пользователя. Поскольку я могу ожидать, в этом случае, что возвращаемое значение может быть чем-то другим, чем я ожидаю, я бы не рассматривал это по-настоящему исключительное поведение.
Кроме того, ваш прецедент, который приводит к дополнительному значению перечисления, не должен вводить ошибку в ваш конкретный метод. Мое предположение состояло бы в том, что прецедент применяется только при входе в систему - что произойдет до того, как метод ChangePassword может быть юридически вызван, предположительно, поскольку вы хотите убедиться, что человек был зарегистрирован до изменения пароля - вы должны никогда не видеть значение ContactUs как возврат от проверки пароля. В случае использования будет указано, что если результатом ContactUs является возврат, эта проверка подлинности завершается с ошибкой до тех пор, пока условие, приводящее к результату, не будет очищено. Если бы это было иначе, я ожидал бы, что у вас будут требования относительно того, как другие части системы должны реагировать в этом состоянии, и вы должны писать тесты, которые заставили бы вас изменить код в этом методе, чтобы соответствовать этим тестам и укажите новое возвращаемое значение.
Проверка ограничений времени выполнения обычно хорошая, поэтому да, лучшая практика, помогает с принципом "сбой", и вы останавливаетесь (или регистрируетесь) при обнаружении ошибки, а не в молчании. Бывают случаи, когда это неверно, но, учитывая операторы switch, я подозреваю, что мы не видим их очень часто.
Чтобы разработать, операторы switch всегда могут быть заменены блоками if/else. Посмотрев на это с этой точки зрения, мы видим, что бросок vs не выбрасывает случай переключения по умолчанию, эквивалентный этому примеру:
if( i == 0 ) {
} else { // i > 0
}
против
if( i == 0 ) {
} else if ( i > 0 ) {
} else {
// i < 0 is now an enforced error
throw new Illegal(...)
}
Второй пример обычно считается лучше, так как вы получаете сбой, когда нарушение ошибки нарушается, а не продолжается при потенциально неправильном допущении.
Если вместо этого мы хотим:
if( i == 0 ) {
} else { // i != 0
// we can handle both positve or negative, just not zero
}
Тогда, на практике я подозреваю, что последний случай, скорее всего, будет выглядеть как оператор if/else. Из-за этого оператор switch так часто напоминает второй блок if/else, что броски обычно являются наилучшей практикой.
Несколько соображений заслуживают внимания:
- рассмотрите полиморфный подход или метод enum для замены оператора switch вообще, например: Методы внутри enum в С#
- если выбрасывание является лучшим, как указано в других ответах, предпочитайте использовать особый тип исключения, чтобы избежать кода плиты котла, например: InvalidEnumArgumentException