Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Struck with Switch, reducing cyclomatic complexity java

Tags:

java

oop

java-8

while working with java i was stuck in a weird situation, lets start with code first.

public static String constructMessage(final String reason, final String xId,
        final String yId, final IonStruct metadataStruct) throws DataSanityException {
    String message = "";
        switch (reason) {
            case "NOT_ACTIVE":
                if (xId != null) {
                    message = String.format("Y %s X %s not active: status is inactive%n", yId, xId);
                } else {
                    message = String.format("Y %s not active: status is inactive%n", yId);
                }
                break;
            case "PRICE_GONE":
                if (metadataStruct.containsKey("applied")) {
                    final String applied = metadataStruct.get("applied");
                    if (applied.equals("single")) {
                        message = String.format("X %s is not active. price for the X is gone. "
                                + "Y price %s for an X should not be gone by %s.%n", xId,
                                metadataStruct.get("yPrice"), metadataStruct.get("zPrice"));
                    } else {
                        if (metadataStruct.get("gonePeriod").equals("gonegone")) {
                            message = String.format("Y price is %s whereas the gone price is %s."
                                    + " Y price for an X should be at least %s%% lower than gone.%n",
                                    metadataStruct.get(yPrice), metadataStruct.get(zPrice), metadataStruct.get("xyzPrice"));
                        } else {
                            message = String.format("Y price %s for an X should be at least %s%% lower than gone price %s in last %s days.%n",
                                    metadataStruct.get(yPrice), metadataStruct.get("mpqPrice"), metadataStruct.get(lpzPrice),
                                    metadataStruct.get("numberOfDays"));
                        }
                    }
                } else {
                    message = String.format(
                            "X %s in Y %s is not active. is %s%%, min mpux is %s%%. ", xId,
                            yId, metadataStruct.get("lpux"), metadataStruct.get("mpxPrice"));
                }
                break;

         and so on ----------- around 20 cases.
         default:
                message = "";
                break;
}

there are 20 cases like this in the complete switch with different messages and different variables used in each of the cases. And the depth of ifs is also upto 3 in some of the switches.

So the cyclomatic complexity is coming 29(too high), so i want to refactor the code with low cyclomatic complexity and better readability.

Any Suggestion ?

like image 355
whishky Avatar asked Apr 02 '26 09:04

whishky


1 Answers

With "use enums instead", Kayaman means something like:

interface MessageGenerator {
    String getMessage(String xId, String yId, IonStruct metadataStruct);
}

enum Reason implements MessageGenerator {
    NOT_ACTIVE((xId, yId, unused) -> 
        (xId != null) ? String.format("Y %s X %s not active: status is inactive%n", yId, xId)
                      : String.format("Y %s not active: status is inactive%n", yId)
    }),
    PRICE_GONE(...),
    DEFAULT(xId, yId, metadataStruct) -> 
        String.format("X %s in Y %s is not active. is %s%%, min mpux is %s%%. ", 
        xId, yId, metadataStruct.get("lpux"), metadataStruct.get("mpxPrice")));

    private MessageGenerator delegate;
    private Reason(MessageGenerator delegate) {
        this.delegate = delegate;
    }
    // implement MessageGenerator interface by delegation
    public String getMessage(String xId, String yId, IonStruct metadataStruct) {
        return delegate.getMessage(xId, yId, metadataStruct);
    }

    public static Reason findReason(String reason) {
        Reason result = DEFAULT;
        try {
            result = valueOf(reason);
        } catch (RuntimeException ex) {}
        return result;
    }
}

and you can do

String constructMessage(String reason, final String xId,
    final String yId, final IonStruct metadataStruct) {
    Reason r = Reason.findReason(reason);
    return r.getMessage(xId, yId, metadataStruct);
}
like image 143
daniu Avatar answered Apr 03 '26 22:04

daniu