I try to simplify the context. I have to improve that kind of code. Please consider this enum :

public enum ErrorColor {

YELLOW_VALUE_IS_MISSING("The value for field yellow is missing"),
RED_VALUE_IS_MISSING("The value for field red is missing"),
BLUE_VALUE_IS_MISSING("The value for field blue is missing"),
GREEN_VALUE_IS_MISSING("The value for field green is missing"),
PURPLE_VALUE_IS_MISSING("The value for field purple is missing");

ErrorColor(String message) {this.message = message;}

private String message;

public String getMessage() {return message;}  

}

Now this use in another class:

    private String defineColorLevel(String color, String yellow, String red, String blue, String green, String purple) throws Exception {
    switch(color){
        case "yellow" : {
            if (yellow == null)
                throw new Exception(ErrorColor.YELLOW_VALUE_IS_MISSING.getMessage());
            return yellow;
        }
        case "red" : {
            if (red == null)
                throw new Exception(ErrorColor.RED_VALUE_IS_MISSING.getMessage());
            return red;
        }
        case "blue" : {
            if (blue == null)
                throw new Exception(ErrorColor.BLUE_VALUE_IS_MISSING.getMessage());
            return blue;
        }
        case "green" : {
            if (green == null)
                throw new Exception(ErrorColor.GREEN_VALUE_IS_MISSING.getMessage());
            return green;
        }
        case "purple" : {
            if (purple== null)
                throw new Exception(ErrorColor.PURPLE_VALUE_IS_MISSING.getMessage());
            return purple;
        }
        default: throw new Exception("UnexpectedError");
    }
}

Actually, I have more than 15 conditions like these one and Sonar is not happy at all... Could you please suggest a way to avoid this 15 if suite ?

I tried a method like

private throwErrorIfValueIsNullOrReturnIt(String value){
if(value == null)
throw new Exception(Error.colorIsMissing(value))
return value;
}

with the corresponding method in enum to build the message but... value is null !!! ^^ So I can't use it to match with the corresponding missing input, neither to build the message... And I can't use the name of the String variable... Can I ?

Or... Should I let sonar cry ?

CodePudding user response：

Use the power of Objects

Basically you need to combine the type of color "yellow", "red", "blue", etc. and its value in order to avoid redundant logic.

For that, you can define a class or a Java 16 record. I'll go with the latter option:

public record ColorValue(String value, ColorType type) {}

Where ColorType is your enum. Yes, you can the same enum to classify the types of colors and hold the error message. Therefore ColorType is more descriptive name.

public enum ColorType {
    
    YELLOW("The value for field yellow is missing"),
    RED("The value for field red is missing"),
    BLUE("The value for field blue is missing"),
    ... etc.

    // the rest code without changes
}

And your method that responsible for performing validation might look like this:

private String throwErrorIfValueIsNullOrReturnIt(ColorValue color) throws MyException {
    if (color.value() == null)
        throw new MyException(color.type().getMessage());
    
    return color.value();
}

Note: avoid using Exception type, try to use the most specific exception subtypes suitable for your case, or create a custom exception.

CodePudding user response：

The error messages differ only in color name. I would create a function to build the error message dynamically:

public class ErrorMessageBuilder implements Function<String, String> {

    @Override
    public String apply(String colorName) {
        return String.format("The value for field %s is missing", colorName);
    }
}

You can cache the function somewhere appropriate, it is stateless. Then simplify defineColorLevel to have only 2 parameters, color and value.

private String defineColorLevel(String color, String value) throws Exception {
    ErrorMessageBuilder errorMessageBuilder = new ErrorMessageBuilder();
    //validate color somehow, if needed at all
    if (value == null) {
        throw new Exception(errorMessageBuilder.apply(color));
    }
    return value;
}

Additionally, to guard against invalid colors, you can define an enum with valid ones, or a Set, or something else depending on exact use case.