Бросать исключения в операторах switch, когда ни один указанный случай не может быть обработан

Question

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

Это только я, или это default: throw new Exception() хорошая идея? Я видел это несколько лет назад и всегда (после этого) считал, что это лучшая практика.

Answer 1

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

Answer 2

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

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

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

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

Answer 3

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

Answer 4

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

Answer 5

Проверка ограничений во время выполнения - это, как правило, хорошая вещь, так что да, передовая практика помогает с принципом 'fast fast', и вы останавливаетесь (или ведете журнал) при обнаружении состояния ошибки, а не продолжаете молча.Есть случаи, когда это не так, но, учитывая подозрительные операторы switch, я подозреваю, что мы не видим их очень часто.

Для уточнения операторы switch всегда можно заменить на блоки if / else.Глядя на это с этой точки зрения, мы видим, что бросок против не бросать в случае переключателя по умолчанию примерно эквивалентен этому примеру:

    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 в C # - если бросание является лучшим, то какотмеченные в других ответах предпочитают использовать определенный тип исключения, чтобы избежать кода котельной плиты, например: InvalidEnumArgumentException

Answer 6

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

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

Answer 7

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

Answer 8

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

Answer 9

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