Подтвердить что ты не робот

Выбрасывание исключений в операторах 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 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() хорошая идея? Я видел это несколько лет назад, и всегда (после того, как он это сделал) предположил, что это лучшая практика.

4b9b3361

ОТВЕТЫ

Ответ 1

Я всегда делаю исключение в этом случае. Подумайте, используя InvalidEnumArgumentException, который дает более подробную информацию в этой ситуации.

Ответ 2

С тем, что у вас есть, это хорошо, хотя инструкция break после нее никогда не будет удалена, потому что выполнение этого потока прекратится, когда исключение будет отправлено и необработано.

Ответ 3

Я использовал эту практику раньше для конкретных случаев, когда перечисление живет "далеко" от нее, но в тех случаях, когда перечисление действительно близко и посвящено определенной функции, это кажется немного немного.

По всей видимости, я подозреваю, что отказ от отладки, вероятно, более уместен.

http://msdn.microsoft.com/en-us/library/6z60kt1f.aspx

Обратите внимание на второй пример кода...

Ответ 4

Мне всегда нравится делать то, что у вас есть, хотя обычно я вывожу ArgumentException, если это аргумент, который был передан, но я вроде как NotImplementedException лучше, так как ошибка, скорее всего, а не вызывающий должен изменить аргумент на поддерживаемый.

Ответ 5

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

Ответ 6

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

Ответ 7

Вы заявляете, что находитесь в очень оборонительной ситуации, и я могу сказать, что это за борт. Разве другой разработчик не будет проверять свой код? Разумеется, когда они проводят простейшие тесты, они поймут, что пользователь все равно может войти в систему, поэтому они поймут, что им нужно исправить. То, что вы делаете, не ужасно или неправильно, но если вы тратите много времени на это, это может быть слишком много.

Ответ 8

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

Кроме того, ваш прецедент, который приводит к дополнительному значению перечисления, не должен вводить ошибку в ваш конкретный метод. Мое предположение состояло бы в том, что прецедент применяется только при входе в систему - что произойдет до того, как метод ChangePassword может быть юридически вызван, предположительно, поскольку вы хотите убедиться, что человек был зарегистрирован до изменения пароля - вы должны никогда не видеть значение ContactUs как возврат от проверки пароля. В случае использования будет указано, что если результатом ContactUs является возврат, эта проверка подлинности завершается с ошибкой до тех пор, пока условие, приводящее к результату, не будет очищено. Если бы это было иначе, я ожидал бы, что у вас будут требования относительно того, как другие части системы должны реагировать в этом состоянии, и вы должны писать тесты, которые заставили бы вас изменить код в этом методе, чтобы соответствовать этим тестам и укажите новое возвращаемое значение.

Ответ 9

Проверка ограничений времени выполнения обычно хорошая, поэтому да, лучшая практика, помогает с принципом "сбой", и вы останавливаетесь (или регистрируетесь) при обнаружении ошибки, а не в молчании. Бывают случаи, когда это неверно, но, учитывая операторы 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