Lancer des exceptions dans les instructions de commutation lorsqu'aucun cas spécifié ne peut être traité

Question

J'écris du C# depuis 8 ans et je suis devenu un programmeur OOP assez défensif. En travaillant dans un langage à typage statique, vous faites des choses comme valider les arguments dans les méthodes et lancer des exceptions, ce que vous ne feriez pas dans des langages dynamiques et plus libéraux. Je me considère comme un expert en C# et je cherche à obtenir des commentaires d'autres développeurs C# expérimentés sur les meilleures pratiques.

Disons que nous avons une fonction qui change le mot de passe d'un utilisateur dans un système dans une application 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() renvoie un UserValidationResult qui est défini comme suit :

enum UserValidationResult
{
    BadUsername,
    BadPassword,
    TrialExpired,
    Success
}

Étant un programmeur défensif, je modifierais le texte ci-dessus ChangePassword() pour lever une exception en cas d'utilisation d'une méthode non reconnue. UserValidationResult valeur de retour de 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
}

J'ai toujours considéré un modèle comme le dernier extrait ci-dessus comme une meilleure pratique. Par exemple, que se passe-t-il si un développeur obtient une exigence selon laquelle, lorsque les utilisateurs essaient de se connecter, s'ils ont telle ou telle raison professionnelle, ils doivent d'abord contacter l'entreprise ? Alors UserValidationResult La définition de l'UE est mise à jour :

enum UserValidationResult
{
    BadUsername,
    BadPassword,
    TrialExpired,
    ContactUs,
    Success
}

Le développeur modifie le corps du fichier ValidateLogin() pour retourner la nouvelle valeur de l'enum ( UserValidationResult.ContactUs ) lorsqu'il est applicable, mais oublie de mettre à jour ChangePassword() . Sans l'exception dans le commutateur, l'utilisateur est toujours autorisé à changer son mot de passe alors que sa tentative de connexion ne devrait même pas être validée en premier lieu !

C'est juste moi, ou c'est default: throw new Exception() une bonne idée ? Je l'ai vu il y a quelques années et j'ai toujours considéré (après l'avoir appris) qu'il s'agissait d'une bonne pratique.

Answer 1

Je lève toujours une exception dans ce cas. Envisagez d'utiliser InvalidEnumArgumentException qui donne des informations plus riches dans cette situation.

Answer 2

Avec ce que vous avez, tout va bien, même si l'instruction break qui suit ne sera jamais exécutée, car l'exécution de ce thread s'arrêtera lorsqu'une exception sera levée et non gérée.

Answer 3

J'ai déjà utilisé cette pratique pour des cas spécifiques où l'énumération vit "loin" de son utilisation, mais dans les cas où l'énumération est vraiment proche et dédiée à une fonctionnalité spécifique, cela semble un peu trop.

Selon toute vraisemblance, je pense qu'un échec d'assertion de débogage est probablement plus approprié.

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

Notez le deuxième exemple de code...

Answer 4

J'aime toujours faire exactement ce que tu as fait, bien que j'ai l'habitude de jeter un ArgumentException si c'est un argument qui a été transmis, mais j'aime bien NotImplementedException C'est mieux puisque l'erreur indique probablement qu'une nouvelle déclaration de cas doit être ajoutée plutôt que l'appelant doit changer l'argument pour un argument supporté.

Answer 5

Je n'ajoute jamais de break après une instruction throw contenue dans une instruction switch. Le problème, et non des moindres, est l'ennuyeux "code inaccessible détecté" avertissement. Alors oui, c'est une bonne idée.