The Cyclomatic Complexity of this method is greater than authorized

Shiladittya Chakraborty picture Shiladittya Chakraborty · May 5, 2016 · Viewed 16.8k times · Source

I am using below method for checking null or empty field:

public boolean isVoNotNull(){
        return null != this.cardNo &&  StringUtils.isNotBlank(this.cardNo) 
                && null != this.otp && StringUtils.isNotBlank(this.otp) 
                && null != this.password && StringUtils.isNotBlank(this.password)
                && null != this.userid && StringUtils.isNotBlank(this.userid) 
                && null != this.type && StringUtils.isNotBlank(this.type) 
                && null != this.walletMobileNo && StringUtils.isNotBlank(this.walletMobileNo);
    }

But getting below exception while this code is validating with SonarLint

EXCEPTION : The Cyclomatic Complexity of this method "isVoNotNull" is 12 which is greater than 10 authorized.

How can I solve this exception or how can I remove the complexity from the code?

Answer

BalusC picture BalusC · May 5, 2016

You need to analyze duplicated code and refactor it into a reusable method.

Given your original snippet,

public boolean isVoNotNull() {
    return null != this.cardNo &&  StringUtils.isNotBlank(this.cardNo) 
        && null != this.otp && StringUtils.isNotBlank(this.otp) 
        && null != this.password && StringUtils.isNotBlank(this.password)
        && null != this.userid && StringUtils.isNotBlank(this.userid) 
        && null != this.type && StringUtils.isNotBlank(this.type) 
        && null != this.walletMobileNo && StringUtils.isNotBlank(this.walletMobileNo);
}

we can identify the following repetitive part:

null != this.xxx && StringUtils.isNotBlank(this.xxx)

Given that StringUtils#isNotBlank() already checks for null, we can further simplify it.

StringUtils.isNotBlank(this.xxx)

Given that you need to invoke this a variable number of times, best would be to refactor it to a method taking a variable number of arguments which checks them all in a loop.

public static boolean isNoneBlank(String... strings) {
    for (String string : strings) {
        if (!StringUtils.isNotBlank(string)) {
            return false;
        }
    }
    return true;
}

Or if you're already on Java 8 with Streams and Lambda support:

public static boolean isNoneBlank(String... strings) {
    return Arrays.stream(strings).allMatch(StringUtils::isNotBlank);
}

Now you can make use of it as below:

public boolean isVoNotNull() {
    return isNoneBlank(this.cardNo, this.otp, this.password, this.userid, this.type, this.walletMobileNo);
}

You could further reduce the boilerplate by removing the unnecessary this.

public boolean isVoNotNull() {
    return isNoneBlank(cardNo, otp, password, userid, type, walletMobileNo);
}

This all was an appliance of the Don't Repeat Yourself (DRY) software engineering principle.

That said, as msandiford pointed out, it turns out that Apache Commons Lang StringUtils has since version 3.2 already exactly this method. So if you don't have it yet, consider upgrading Apache Commons Lang to at least 3.2.

public boolean isVoNotNull() {
    return StringUtils.isNoneBlank(cardNo, otp, password, userid, type, walletMobileNo);
}