Выдавание исключения в операторах переключения, когда никакой указанный случай не может быть обработан
Скажем, у нас есть функция, которая изменяет пароль для пользователя в системе в приложении 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
}
Я всегда рассматривал шаблон как последний отрывок выше лучшей практики. Например, что, если один разработчик получает требование, которые теперь, когда пользователи пытаются войти в систему, если по this-that бизнес-причине, они, предполагают для контакта с бизнесом сначала? Так UserValidationResultопределение обновляется, чтобы быть:
enum UserValidationResult
{
BadUsername,
BadPassword,
TrialExpired,
ContactUs,
Success
}
Разработчик изменяет тело ValidateLogin() метод для возврата нового перечисления значений (UserValidationResult.ContactUs) когда применимо, но забывает обновлять ChangePassword(). Без исключения в переключателе пользователю все еще разрешают изменить их пароль, когда их попытка входа в систему не должна даже быть проверена во-первых!
Это - просто я или является этим default: throw new Exception() хорошая идея? Я видел, что это несколько лет назад и всегда (после groking это) предполагает, что это лучшая практика.
8 ответов
В этом случае я всегда генерирую исключение. Рассмотрите возможность использования исключения InvalidEnumArgumentException , которое дает более обширную информацию в этой ситуации.
Я использовал эту практику раньше для конкретных случаев, когда перечисление живет «далеко» от своего использования, но в тех случаях, когда перечисление действительно близко и посвящено конкретной функции, оно кажется небольшим многовато.
По всей вероятности, я подозреваю, что ошибка подтверждения отладки, вероятно, более уместна.
http://msdn.microsoft.com/en-us/library/6z60kt1f.aspx
Обратите внимание на второй пример кода ...
Мне всегда нравится делать именно то, что есть у вас, хотя я обычно генерирую ArgumentException , если это переданный аргумент, но мне нравится NotImplementedException лучше, так как ошибка, скорее всего, состоит в том, что нужно добавить новый оператор case, а не вызывающий должен изменить аргумент на поддерживаемый.
Вы утверждаете, что занимаетесь оборонительной обороной, и я почти могу сказать, что это перебор. Разве другой разработчик не собирается тестировать их код? Конечно, когда они проведут простейшие тесты, они увидят, что пользователь все еще может войти в систему, и тогда они поймут, что им нужно исправить. То, что вы делаете, не является ужасным или неправильным, но если вы тратите на это много времени, этого может быть слишком много.
Для веб-приложения я бы предпочел значение по умолчанию, которое генерирует результат с сообщением об ошибке, которое просит пользователя связаться с администратором и регистрирует ошибку, а не генерирует исключение, которое может привести к чему-то меньшему. значимый для пользователя. Поскольку в этом случае я могу ожидать, что возвращаемое значение может быть чем-то другим, чем я ожидал, я бы не стал рассматривать это действительно исключительное поведение.
Кроме того, ваш вариант использования, который приводит к дополнительному значению перечисления, не должен приводить к ошибке в вашем конкретном методе.Я ожидал, что этот вариант использования будет применяться только при входе в систему - что произойдет до того, как метод ChangePassword может быть вызван на законных основаниях, предположительно, поскольку вы хотите убедиться, что человек вошел в систему, прежде чем менять свой пароль - вам следует никогда не воспринимайте значение ContactUs как результат проверки пароля. Вариант использования должен указывать, что если возвращается результат ContactUs, эта аутентификация не выполняется до тех пор, пока условие, приводящее к результату, не будет очищено. Если бы все было иначе, я бы ожидал, что у вас будут требования к тому, как другие части системы должны реагировать в этом состоянии, и вы бы написали тесты, которые заставили бы вас изменить код в этом методе, чтобы он соответствовал этим тестам и адрес новое возвращаемое значение.
С тем, что у вас есть, все в порядке, хотя оператор break после него никогда не будет выполнен, потому что выполнение этого потока прекратится, когда исключение будет сгенерировано и не обработано.
Я думаю, что подобные подходы могут быть очень полезными (например, см. Ошибочно пустые пути кода ). Еще лучше, если вы можете скомпилировать этот default-throw в выпущенном коде, например assert. Таким образом, у вас будет дополнительная защита во время разработки и тестирования, но вы не будете нести лишние накладные расходы на код при выпуске.
I никогда не добавляйте разрыв после оператора throw, содержащегося в операторе switch. Не последнюю из проблем вызывает раздражающее предупреждение «Обнаружен недоступный код» . Так что да, это хорошая идея.