What is the best way to make this code more object oriented?

Question

I'm new to Java and object oriented design concepts. I have a piece of code that I know could be improved, but I'm not sure the best way to go about it.

I have a stream of data (authorization values) coming in that I am trying to convert to a map of authorization ID to validity of the value. In order to test the validity, I need to check the type of authorization value it is, and send it to a remote client to make sure it works.

I am creating the map of authorization validity here:

private Map<UUID, Boolean> getAuthorizationValidityMap(UUID accountId) {
        final Map<UUID, Boolean> authValidity = findCredentialsByAccountId(accountId)
                .parallelStream()
                .map((auth -> Pair.of(auth.getId(), isAuthValid(auth))))
                .collect(Collectors.toMap(res -> res.getLeft(), res -> res.getRight()));
        return authValidity;
    }

The function to find if the authorization value is valid. There are multiple authorization value types, and each one needs to check the validity differently. The way I have it implemented feels clunky because as more auth types get added, it will just result in more else if statements.

private Boolean isAuthValid(AuthorizationValue authValue) {
        if (authValue.getType().equals(AUTH_TYPE_A)) {
            return isAuthTypeAValid(authValue);
        } else if (authValue.getType().equals(AUTH_TYPE_B)) {
            return isAuthTypeBValid(authValue);
        } else if (authValue.getType().equals(AUTH_TYPE_C)) {
            return isAuthTypeCValid(authValue);
        } 
        return false;
    }

Not so much about making it more object-oriented... but is `authValue.getType()` an enum? If so, a switch would be better than if/else. — Andy Turner, Jul 01 '22 at 15:03
This may not be the answer but in first place, code should be "object oriented". It should be readable and reusable. If this can be achieved with a simpler approach, then choose the simple approach. — David Weber, Jul 01 '22 at 15:10
To that, if you are new to Java - do you understand the code that you posted? Understand that much, that you could write it without lambda expressions? — David Weber, Jul 01 '22 at 15:12
@DavidWeber Thanks for the replies. Could you explain more what you mean? What are some improvements you would make? — Billy Keef, Jul 01 '22 at 15:40
You could use inheritance where the subclass are different type of Authorization and each subclass overrides isAuthValid(). Then make findCredentialsByAccountId(accountId) returns specific type of Authorization object. Then instead of isAuthValid(auth), simply auth.isAuthValid(). — Cheng Thao, Jul 01 '22 at 15:53
@Billy Keef: If you want to do this "more" object oriented, thenn Cheng Thao's answer is your way to go. My point is that the code is readable as it is and you should not destroy it because you want it to be more object oriented. Values have to be compared and somewhere the results of the comparisons ALWAYS have to be defined. If you are bothered by the two methods, then you could write a functional interface (with a method that returns a boolean), and use that instead of isAuthValid(auth). Then the validation is defined directly in the getAuthorizationValidityMap() method. — David Weber, Jul 01 '22 at 17:49
@Billy Keef: But then it must be done every time getAuthorizationValidityMap() is called. — David Weber, Jul 01 '22 at 17:49

score 0 · Answer 1 · answered Jul 05 '22 at 07:52

At runtime you can decide what Authorize object should be used to check your credentials. It can be done through Strategy pattern. Strategies of authorization will be stored in collection and you can get instance of authorization by using Factory pattern.

So let's see an example of implementation. I will show an example via C#, however I've placed comments what data structure can be used in Java. This is property types of Auth:

public enum AuthType
{ 
    A, B, C, D
}

And then you can create abstract class Authorization which will define behaviour for all derived classes. These derived classes will auth values:

public abstract class Authorization 
{
    public abstract bool IsValid(string value);
}

public class Authorization_A : Authorization
{
    public override bool IsValid(string value)
    {
        // null check should be implemented
        return true;
    }  
}

public class Authorization_B : Authorization
{
    public override bool IsValid(string value)
    {
        // null check should be implemented
        return true;
    }
}

And we need a factory which will return instance of authorization by AuthType:

public class AuthorizationFactory 
{
    // HashTable in Java
    private Dictionary<AuthType, Authorization> _authByType
        = new Dictionary<AuthType, Authorization>()
    {
        { AuthType.A, new Authorization_A() },
        { AuthType.B, new Authorization_B() }
    };

    public Authorization GetInstance(AuthType authType)
    { 
       return _authByType[authType];
    }
}

And this is a mock method of getting auth type:

AuthType GetAuthType() 
{
    // your logic here of getting AuthType
    return AuthType.A;
}

And a method which uses factory:

bool IsAuthValid(AuthType authType) 
{
    AuthorizationFactory authorizationFactory = new AuthorizationFactory();
    Authorization authorization = authorizationFactory.GetInstance(authType);
    return authorization.IsValid("some value");
}

And you can call it like this:

AuthType authType = GetAuthType();
bool isAuthValid = IsAuthValid(authType);

So we are getting authorization instance to check based on AuthType property. And the pattern name is Strategy.

So our code follows Open Closed principle of SOLID principles. When you will want to add new strategy of authorization you will need a new class of FooAuthorization.

What is the best way to make this code more object oriented?

1 Answers1