Выбрасывание исключений в операторах switch, когда не может быть обработан ни один указанный случай
допустим, у нас есть функция, которая изменяет пароль для пользователя в системе в приложении 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, которое определяется как:
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's определение обновляется, чтобы быть:
enum UserValidationResult
{
BadUsername,
BadPassword,
TrialExpired,
ContactUs,
Success
}
разработчик изменяет тело ValidateLogin() метод для возврата нового значения перечисления (UserValidationResult.ContactUs) когда применимо, но забывает обновить ChangePassword(). Без исключения в переключатель, пользователь может изменить свой пароль при попытке входа не должно быть проверено в первую очередь!
это только я, или это default: throw new Exception() хорошая идея? Я видел это несколько лет назад и всегда (после того, как грокнул его) предполагаю, что это лучшая практика.
9 ответов:
Я всегда бросаю исключение в этом случае. Рассмотрите возможность использования
InvalidEnumArgumentException, что дает более богатую информацию в этой ситуации.
с тем, что у вас есть, это нормально, хотя оператор break после него никогда не будет поражен, потому что выполнение этого потока прекратится, когда исключение будет брошено и необработано.
Я использовал эту практику раньше для конкретных случаев, когда перечисление живет "далеко" от его использования, но в случаях, когда перечисление действительно близко и посвящено конкретной функции, это кажется немного большим.
по всей вероятности, я подозреваю, что ошибка утверждения отладки, вероятно, более уместна.
http://msdn.microsoft.com/en-us/library/6z60kt1f.aspx
обратите внимание на второй пример кода...
мне всегда нравится делать именно то, что у вас есть, хотя я обычно бросаю
ArgumentExceptionЕсли это аргумент, который был передан, но мне вроде какNotImplementedExceptionлучше, так как ошибка, вероятно, что новый оператор case должен быть добавлен, а не вызывающий должен изменить аргумент на поддерживаемый.
Я никогда не добавляю перерыв после оператора throw, содержащегося в операторе switch. Не последняя из проблем является раздражающим "обнаружен недостижимый код" предупреждение. Так что да, это хорошая идея.
Я думаю, что подобные подходы могут быть очень полезны (например, см. ошибочно пустые пути код). Еще лучше, если вы можете скомпилировать этот бросок по умолчанию в выпущенном коде, например assert. Таким образом, вы получаете дополнительную защиту во время разработки и тестирования, но не несете дополнительных затрат кода при выпуске.
вы утверждаете, что вы очень защищаетесь, и я мог бы почти сказать, что это за бортом. Разве другой разработчик не собирается протестировать свой код? Конечно, когда они делают самые простые тесты, они увидят, что пользователь все еще может войти в систему - так что они поймут, что им нужно исправить. То, что вы делаете, не ужасно или неправильно, но если вы тратите много времени на это, это может быть слишком много.
для веб-приложения я бы предпочел значение по умолчанию, которое генерирует результат с сообщением об ошибке, которое просит пользователя связаться с администратором и регистрирует ошибку, а не создает исключение, которое может привести к чему-то менее значимому для пользователя. Поскольку я могу предвидеть, что в этом случае возвращаемое значение может быть чем-то иным, чем я ожидаю, я бы не рассматривал это действительно исключительное поведение.
кроме того, ваш вариант использования, который приводит к дополнительному значению перечисления не следует вводить ошибку в ваш конкретный метод. Я ожидаю, что вариант использования будет применяться только при входе в систему-что произойдет до того, как метод ChangePassword может быть законно вызван, предположительно, поскольку вы хотите убедиться, что человек вошел в систему перед изменением своего пароля-вы никогда не должны видеть значение ContactUs как возврат от проверки пароля. В случае использования будет указано, что если результат ContactUs возвращается, то аутентификация не выполняется до тех пор, пока условие, приводящее к результату, очищается. Если бы это было иначе, я ожидал бы, что у вас будут требования к тому, как другие части системы должны реагировать при этом условии, и вы напишете тесты, которые заставят вас изменить код в этом методе, чтобы соответствовать этим тестам и адресовать новое возвращаемое значение.
проверка ограничений времени выполнения обычно является хорошей вещью, поэтому да, лучшая практика, помогает с принципом "fail fast", и вы останавливаетесь (или регистрируете) при обнаружении состояния ошибки, а не продолжаете молча. Есть случаи, когда это не так, но, учитывая заявления switch, я подозреваю, что мы не видим их очень часто.
чтобы уточнить, операторы switch всегда могут быть заменены блоками if/else. Глядя на это с этой точки зрения, мы видим, что бросок vs не бросает в a случай переключателя по умолчанию примерно эквивалентен этому примеру:
if( i == 0 ) { } else { // i > 0 }vs
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 так часто напоминает во-вторых, если/else блок, что броски, как правило, лучшая практика.
еще несколько соображений стоит: - рассмотрим полиморфный подход или метод перечисления, чтобы полностью заменить оператор switch, например:методы внутри перечисления в C# - если метание является лучшим, как отмечено в других ответах, предпочитают использовать определенный тип исключения, чтобы избежать кода котельной плиты, например:
InvalidEnumArgumentException