The Cyclomatic Complexity of this method is greater than authorized

Question

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 1

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);
}

Answer 2

As others have pointed out the null check is redundant as isNotBlank checks this too.

So, if you are using Apache StringUtils 3.2 or later, you can just use StringUtils.isNoneBlank(...)

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

If you are using an earlier version, you could easily write one:

public static boolean isNoneBlank(CharSequence... seqs) {
  for (CharSequence seq : seqs) {
    if (StringUtils.isBlank(seq))
      return false;
  }
  return true;
}

Or in Java 8:

public static boolean isNoneBlank(CharSequence... seqs) {
  return Stream.of(seqs).allMatch(StringUtils::isNotBlank);
}

Answer 3

Add the strings to an array and check with a loop for nullity and not blank-ness. Worst performance of course but should break that non-sensed limit.

Example code:

public boolean isVoNotNull(){
    string [] stringsToCheck = new string[]{
    this.cardNo,
    this.otp,
    // ...
    }

    //return CheckStringArrayNotNull(stringsToCheck);        
    return StringUtils.isNoneBlank(stringsToCheck);
}

// high chance this method already exists inside StringUtils
// INFACT FOUND! :)
private boolean CheckStringArrayNotNull( string [] stringsToCheck){
     for(int i=0; i<stringsToCheck.length(); i++)
        if( StringUtils.isNotBlank( stringsToCheck[i]) == false)
            return false;

    return true; 
}

or even better, (EDIT: seems @msandiford beat me on time for this solution)

public boolean isVoNotNull(){      
    return StringUtils.isNoneBlank(cardNo, otp, ...);
}

You could of course break into more methods to reduce cyclomatic complexity but that makes the class even harder to read and understand. Code metrics should be used only to investigate problems, not as a development mantra, for example I can take a very stupid class with 100 methods each with Cyclomatic complexity of 100 and refactor it so that it has perfect code metrics but is impossible to understand. Never look at code metrics, think to good solutions without using coding metrics.