Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Returning different types from a C# method

Tags:

c#

oop

I have a method:

public ??? AuthManager.Login(Credentials credentials)

Here is a set of valid output values of this method:

  1. Success (+accountId)
  2. Failure: AccountLockedOut
  3. Failure: UsernameNotFound
  4. Failure: InvalidPassword (+failed attempt count)

Depending on the return type different views are shown to the user (yes, view for AccountLockedOut is different from InvalidPassword).

I could go with:

public class LoginAttemptResult {
    public bool Succeeded { get; set; }
    public AccountId AccountId { get; set; } // for when success
    public LoginAttemptResultEnumType Result { get;set; } // Success, Lockedout, UsernameNotFound, InvalidPassword  
    public int FailedAttemptCount { get; set; } // only used for InvalidPassword
}

I don't like this and looking for a better solution. First, this results in a partially initialized object, two it violates interface segregation principle, three it violates SRP.

UPDATE: throwing exceptions is also not an elegant solution because InvalidPassword as I see it is not an exception. Failed DB connection is an exception. Null argument is an exception. InvalidPassword is a valid anticipated response.

I think better solution is to create a hierarchy of classes:

abstract class LoginAttemptResult
    sealed class LoginSuccess : LoginAttemptResult { AccountId }
    abstract class LoginFailure : LoginAttemptResult
        sealed class InvalidPasswordLoginFailure : LoginFailure { FailedAttemptCount }
        sealed class AccountLockedoutLoginFailure : LoginFailure

the caller of Login method then would have to do something like:

if (result is LoginSuccess) { 
    ..."welcome back mr. account id #" + (result as LoginSuccess).AccountId
}
else if (result is InvalidPasswordLoginFailure ) { 
    ..."you failed " + (result as InvalidPasswordLoginFailure).FailedAttemptCount + " times"
}

I don't see anything wrong (conceptually) with this approach (other than a number of classes it comes with).

What else is wrong with this approach?

Notice, this approach is essentially an F#'s discriminated union (DU) .

Is there a better way to model this? I already have several solutions that work - now I want an elegant solution that works.

like image 414
THX-1138 Avatar asked May 03 '13 15:05

THX-1138


1 Answers

I think your solution is OK in the case if result classes differs significantly and you need a separate class for each. But I'm not sure about that. Try this class for each result:

/// <summary>
/// Immutable, created by the server
/// </summary>
class LoginResult
{
    /// <summary>
    /// Null in the case of failure
    /// </summary>
    public int? Id { get; private set; }

    /// <summary>
    /// Null in the case of success
    /// </summary>
    public string FailReason { get; private set; }

    /// <summary>
    /// Always >= 1
    /// </summary>
    public int AttemptNumber { get; private set; }

    public LoginResult(int id, int attemptNumber)
    {
        Id = id;
        AttemptNumber = attemptNumber;
    }

    public LoginResult(string reason, int attemptNumber)
    {
        FailReason = reason;
        AttemptNumber = attemptNumber;
    }
}

I can imagine, that your authentication logic can be very complicated, and Id, FailReason and AttemptNumber are not only properties you'll need. In this case you need to present us more concrete example, we'll try to build abstractions that will fit your logic, if neccessary. In this particular case - no sense for abstraction.

like image 159
astef Avatar answered Oct 19 '22 06:10

astef