I have a method:
public ??? AuthManager.Login(Credentials credentials)
Here is a set of valid output values of this method:
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.
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With