Проверить значение контрольного параметра или вернуть bool? - PullRequest
4 голосов
/ 01 сентября 2010

Я использую метод, имеющий следующую подпись:

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

Метод объявляет: bool authenticated = false;, затем продолжается для аутентификации пользователя.

Всякий раз, когда установлен authenticatedв true или false, authenticationFailure устанавливается в AuthenticationFailure.Failure или AuthenticationFailure.Success соответственно.

Таким образом, в основном я могу использовать либо authenticationFailure, либо возвращаемое значение метода для проверки результата.Однако кажется бессмысленным нарушением СУХОГО использовать эти два подхода в одном методе.

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

На данный момент я делаю это:

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;
}

Но я мог бы сделать это и получить аналогичный результат:

public static AuthenticationFailure IsValidLDAPUser(string username, string password, string domain)
{
    var authenticationStatus = new AuthenticationFailure();   
    ActiveDirectoryAuthenticationService.TryAuthenticate(username, password, domain, out authenticationStatus)
    return authenticationStatus;
}
  • Зачем вам ссылочный параметр, который делает то же самое, что и возвращаемое значение?
  • Какой из них я должен использовать для проверки результата и имеет ли это какое-то значение?
  • Этопросто случай плохого кода или я упускаю суть?

Заранее спасибо!

Ответы [ 5 ]

4 голосов
/ 01 сентября 2010

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

Иногда также существует более одного типа успеха - например, HTTP имеет много кодов возврата в блоке 200 и 300, которые в любом случае будут считаться успешными. Таким образом, bool обычно говорит вам, был ли он успешным или нет, а enum дает более точную информацию.

Есть много способов сделать это, и это необычно, но не против DRY, если они планируют добавить больше кодов.

Другой способ - просто инкапсулировать в класс Result, который имеет enum и свойство IsSuccess. Вы даже можете предоставить преобразование в bool, чтобы его было легко использовать в операторах if.

3 голосов
/ 01 сентября 2010

Во-первых, ваше имя переменной enum кажется мне немного искаженным.AuthenticationFailure.Success не имеет особого смысла!Это должно быть AuthenticationResult или что-то еще.Кроме того, поскольку в вашем методе вы просто заботитесь о том, прошла ли аутентификация успешно, вы должны вернуть bool.Вы думаете о СУХОЙ, но также думаете о ПОЦЕЛУЕ!

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

2 голосов
/ 01 сентября 2010

Если функция возвращает только одно значение, то обычное возвращаемое значение функции следует использовать вместо параметра out или ref.

Я бы сказал, что это еще более важно для методов с именем IsXyz (выделение равно ), и методы, названные таким образом, должны возвращать bool.

1 голос
/ 01 сентября 2010

ИМХО, я не думаю, что это бул против реф.Мне кажется, что неправильная вещь возвращается в ссылке.

Я ожидаю увидеть следующие методы

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

и

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

Oneпотому что, когда тебя не волнует, почему и что ты делаешь.например,

Это позволяет вызывающему коду выполнить аутентификацию и затем выполнить что-либо без перехвата

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

Или, если им нужна эта дополнительная информация, тогда используйте перехват

например1016 *

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..

    }
}

Теперь, если нет понятия пользователя или токена, то, вероятно, не нужен Try Pattern

1 голос
/ 01 сентября 2010

Параметры Ref не так просты в использовании и понятны по сравнению с возвращаемым значением. Однако в некоторых ситуациях использование параметров ref может быть преимуществом. В этом случае, если мы предположим, что распределение AuthenticationFailure отнимает много времени, поэтому использование параметра ref подходит, поскольку в случае успешной аутентификации распределение не потребуется.

В любом случае, в этом случае я предпочитаю использовать тип возвращаемого значения AuthenticationResult, так как это не должно влиять на производительность.

...