Refactoring a method having multiple switch case statements & generics

103 Views Asked by At

I have an existing project based on Java in which an existing utility method is present which gets some values as an input and provides result as a boolean value.

I need to refactor this method since there is an upcoming requirement which will cause a greater number of switch cases to be introduced in this method.

I tried other possible solutions shown on other Stack Overflow questions, but those solutions were not applicable for the use case I have here.

The code snippet is mentioned below.

protected static < T extends Comparable < T >> boolean compareValues(T lookupValue, T actualValue, String comparisonCondition, List < T > lookupValues) {
    comparisonCondition = comparisonCondition.toUpperCase();
    boolean result;
    switch (comparisonCondition) {
        case EQUALS:
            result = lookupValue instanceof String && actualValue instanceof String ? (String.valueOf(lookupValue).trim()).equalsIgnoreCase(String.valueOf(actualValue).trim()) : lookupValue.compareTo(actualValue) == 0;
            break;
        case NOT_EQUALS:
            result = lookupValue.compareTo(actualValue) != 0;
            break;
        case LIKE:
            result = StringUtils.containsIgnoreCase(String.valueOf(actualValue), String.valueOf(lookupValue));
            break;
        case NOT_LIKE:
            result = !StringUtils.containsIgnoreCase(String.valueOf(actualValue), String.valueOf(lookupValue));
            break;
        case IN:
            result = lookupValues.stream().anyMatch(lkpValue - > lkpValue instanceof String ? ((String) lkpValue).trim().compareToIgnoreCase(String.valueOf(actualValue).trim()) == 0 : lkpValue.compareTo(actualValue) == 0);
            break;
        case NOT_IN:
            result = lookupValues.stream().noneMatch(lkpValue - > lkpValue instanceof String ? ((String) lkpValue).trim().compareToIgnoreCase(String.valueOf(actualValue).trim()) == 0 : lkpValue.compareTo(actualValue) == 0);
            break;
        default:
            if (LOGGER.isDebugEnabled()) {
                LOGGER.debug(MSG_FORMAT_INVALID_COMPARISON_CONDITION, comparisonCondition);
            }
            result = false;
    }
    if (LOGGER.isDebugEnabled()) {
        LOGGER.debug("Comparing value '{}' with '{}' using comparison condition '{}'.{}Result: {}", actualValue, Objects.nonNull(lookupValue) ? lookupValue : lookupValues.stream().map(Object::toString).collect(Collectors.joining(WhlProcessingConstants.SPLIT_COMMA)), comparisonCondition, LINE_SEPARATOR, result);
    }
    return result;
}

Can you please suggest some solution by which this code of the method can be refactored, so that it is scalable for any future requirements and also, the cognitive complexity will not increase as we include more comparisonConditions / cases in it?

For your information: I am using SonarLint plugin to analyze the cognitive complexity of the code.

2

There are 2 best solutions below

0
On

A very similar example from book Effective Java and after adding generics would work

interface Operation {
    public <lookup,actual> boolean validate(lookup lookupValue, actual actualValue);
}
 enum BasicOperationn implements Operation{
    EQUALS{
        @Override
        public <lookup, actual> boolean validate(lookup lookupValue, actual actualValue) {
          return lookupValue instanceof String && actualValue instanceof String ? (String.valueOf(lookupValue).trim()).equalsIgnoreCase(String.valueOf(actualValue).trim()) :((String) lookupValue).compareTo(((String) actualValue)) == 0;        }
    },
     LIKE {
         @Override
         public <lookup, actual> boolean validate(lookup lookupValue, actual actualValue) {
           return   StringUtils.containsIgnoreCase(String.valueOf(actualValue), String.valueOf(lookupValue));
         }
     };
     ...

}

Edited: Adding client code

class ClientClass{
    protected static < T extends Comparable < T >> boolean compareValues(T lookupValue, T actualValue, String comparisonCondition, List< T > lookupValues) {
        comparisonCondition = comparisonCondition.toUpperCase();
        BasicOperationn operation = BasicOperationn.valueOf(comparisonCondition);
        boolean result = operation.validate(lookupValue, actualValue);
        return result;
    }
}
0
On

what do you think about defining a map with all these predicates and the keys? Additionally, you can try extracting the common comparisons that keep repeating. Here is a possible solution, but you might need to twitch a few things:

private static final Map<String, TriPredicate<T>> comparators = Map.of(
        "EQUALS", (lookup, actual, __) -> lookup instanceof String && actual instanceof String ? compareStrings(lookup, actual) : lookup.compareTo(actual) == 0,
        "NOT_EQUALS", (lookup, actual, __) -> lookup.compareTo(actual) != 0,
        "LIKE", (lookup, actual, __) -> containsIgnoreCase(lookup, actual),
        "NOT_LIKE", (lookup, actual, __) -> !containsIgnoreCase(lookup, actual),
        "IN", (__, actual, lookup) -> listContains(actual, lookup),
        "NOT_IN", (__, actual, lookup) -> !listContains(actual, lookup)
);

protected static <T extends Comparable<T>> boolean compareValues(T lookupValue, T actualValue, String comparisonCondition, List<T> lookupValues) {
    if (LOGGER.isDebugEnabled()) {
        LOGGER.debug("Comparing value '{}' with '{}' using comparison condition '{}'.{}Result: {}", actualValue, Objects.nonNull(lookupValue) ? lookupValue : lookupValues.stream().map(Object::toString).collect(Collectors.joining(WhlProcessingConstants.SPLIT_COMMA)), comparisonCondition, LINE_SEPARATOR, result);
    }
    return comparators.getOrDefault(comparisonCondition.toUpperCase(), (a, b, c) -> false)
            .test(lookupValue, actualValue, lookupValues);
}

private <T extends Comparable> boolean listContains(T actual, List<T> lookup) {
    return lookup.stream().anyMatch(val -> val instanceof String ? compareStrings(val, actual) : val.compareTo(actual) == 0);
}

private boolean containsIgnoreCase(Object actual, Object lookup) {
    return StringUtils.containsIgnoreCase(String.valueOf(actual), String.valueOf(lookup));
}

private boolean compareStrings(Object lookup, Object actual) {
    return (String.valueOf(lookup).trim()).equalsIgnoreCase(String.valueOf(actual).trim());
}


@FunctionalInterface
interface TriPredicate<X> {
    boolean test(X lookup, X actual, List<X> lookupValues);
}