2010-09-01 6 views
4

J'utilise une méthode qui a la signature suivante:Vérifiez la valeur du paramètre de référence ou renvoyez bool?

public static bool TryAuthenticate(string userName, string password, 
    string domainName, out AuthenticationFailure authenticationFailure) 

La méthode déclare: bool authenticated = false; continue ensuite pour authentifier l'utilisateur.

Chaque fois que authenticated est défini sur true ou false, authenticationFailure est défini sur AuthenticationFailure.Failure ou AuthenticationFailure.Success de manière correspondante.

Donc, fondamentalement, je peux utiliser authenticationFailure ou la valeur de retour de la méthode pour vérifier le résultat. Cependant, il semble être une violation inutile de DRY d'avoir ces deux approches dans la même méthode.

Juste pour clarifier, authenticationFailure n'est utilisé nulle part ailleurs dans la méthode, donc il semble être totalement redondant.

En ce moment je fais ceci:

public static bool IsValidLDAPUser(string username, string password, string domain) 
{ 
    var authenticationStatus = new AuthenticationFailure(); 
    if (ActiveDirectoryAuthenticationService.TryAuthenticate(username, password, domain, out authenticationStatus)) 
     return true; 
    else return false; 
} 

Mais je pourrais le faire et obtenir un résultat similaire:

public static AuthenticationFailure IsValidLDAPUser(string username, string password, string domain) 
{ 
    var authenticationStatus = new AuthenticationFailure(); 
    ActiveDirectoryAuthenticationService.TryAuthenticate(username, password, domain, out authenticationStatus) 
    return authenticationStatus; 
} 
  • Pourquoi auriez-vous un paramètre de référence fait la même chose que la valeur de retour?
  • Lequel devrais-je utiliser pour vérifier le résultat et cela fait-il une différence?
  • Est-ce juste un cas de mauvais code ou ai-je manqué le point?

Merci d'avance!

+0

Je pense que votre deuxième bloc de code peut contenir une erreur. Le type de retour ne devrait-il pas être 'bool' au lieu de' AuthenticationFailure' ou devrait-on changer les instructions de retour pour retourner 'authenticationStatus'? –

Répondre

4

Souvent, il y a plus de codes d'erreur que de simples succès ou échecs. Peut-être que le concepteur de cette méthode va ajouter plus d'énumérations pour tous les différents types d'échec.

Parfois, il existe également plus d'un type de succès - par ex. HTTP a beaucoup de codes de retour dans les blocs 200 et 300 qui seraient tous considérés comme un succès d'une manière ou d'une autre. Donc, le bool vous dit généralement si elle a réussi ou non, et l'énumération donne des informations plus précises.

Il y a beaucoup de façons de le faire, et celui-ci est inhabituel, mais pas contre DRY s'ils prévoient d'ajouter plus de codes.

Une autre méthode consiste simplement à encapsuler dans une classe Result qui possède l'enum et une propriété IsSuccess. Vous pourriez même fournir une conversion à bool pour le rendre facile à utiliser dans les instructions if.

+0

Bon point - Je viens d'examiner l'extensibilité de TryAuthenticate et il s'avère que AuthenticationFailure a toutes sortes d'autres valeurs comme AccountDisabled, PasswordExpired, InvalidWorkstation et ainsi de suite ... Donc je vais continuer à vérifier le bool mais peut-être utiliser le AuthenticationFailure lorsque l'authentification ne parvient pas à donner des informations sur les raisons de l'échec. – Nobody

2

Si une fonction renvoie uniquement une valeur, la valeur de retour de la fonction régulière doit être utilisée de préférence à un paramètre out ou ref. Je dirais que c'est encore plus important pour les méthodes nommées IsXyz (l'accent étant mis sur est) et les méthodes nommées de cette manière devraient renvoyer bool.

3

Tout d'abord, votre nom de variable enum me semble un peu tordu. AuthenticationFailure.Success n'a pas beaucoup de sens! Il devrait être AuthenticationResult ou quelque chose. En outre, puisque dans votre méthode vous vous souciez seulement de savoir si l'authentification a réussi, vous devriez retourner bool.Vous pensez DRY mais pensez aussi à KISS!

Vous pouvez toujours utiliser l'énumération pour les autres codes d'erreur que vous pourriez ajouter, mais un coup d'œil à la valeur de retour de votre méthode devrait vous indiquer si elle a réussi ou non. Si vous souhaitez trouver les détails de l'échec, utilisez l'énumération qui a été transmise comme paramètre 'out'.

+0

Oui, j'ai remarqué que le nom de l'énumération n'est pas très bien pensé. Heureusement ce n'est pas moi qui ai fait ce code et je ne le maintiens pas - cela fait partie d'une de ces énormes bibliothèques communes à l'échelle de l'entreprise. – Nobody

1

Les paramètres de référence ne sont pas si faciles à utiliser et comprennent la comparaison avec la valeur de retour. Cependant, il y a une situation où l'utilisation de paramètres ref peut être avantageuse. Dans ce cas, si nous supposons que l'allocation d'AuthenticationFailure prend du temps, l'utilisation du paramètre ref est appropriée car aucune allocation ne sera nécessaire en cas de réussite de l'authentification.

Quoi qu'il en soit, dans ce cas, je préfère utiliser le type de retour AuthenticationResult, car il ne devrait pas y avoir d'impact sur les performances.

1

IMHO Je ne pense pas que ce soit bool vs chose ref. Pour moi, il semble que la mauvaise chose est retourné dans l'arbitre.

j'attendre à voir les méthodes suivantes

public static bool TryAuthenticate(string userName, 
       string password, 
       string domainName, 
       out User user) 

et

public static User Authenticate(string userName, 
       string password,      
       string domainName) 

un pour quand vous ne se soucient pas de savoir pourquoi et qui vous le faites. par exemple.

Cela permet au code d'appel à faire authentifier et faire quelque chose sans une prise

User u; 
if (TryAuthenticate(user,pass,domain,ref u)) 
    //do somthing 
else 
    return; 

Ou si elles ont besoin que d 'info puis utilisez une prise

par exemple

User u; 
try 
{ 
    u = Authenticate(user,pass,domain); 
    //do somthing 
} 
catch(AuthenticationError ae) 
{ 
    switch (ae.Failure) 
    { 
     case AuthenticationFailure.AccountLocked: 
      //dosomthing A 
      break; 
     case AuthenticationFailure.BadPassword: 
      //dosomthing B 
      break; 
     case AuthenticationFailure.InvalidUser: 
      //dosomthing c 
      break; 
     etc.. 

    } 
} 

Maintenant, s'il n'y a pas de concept d'un utilisateur ou d'un jeton alors le modèle d'essai est probablement pas nécessaire

Questions connexes