CODEWITHSUNDEEP
javacoding-style
upintheflame
upintheflame

Reputation: 41

My code is clumsy, sonarqube says "Replace this if-then-else statement by a single return statement."

I'm new to Java. I don't know how to make this code simpler.

if (word == null || word.length() < 3 || !Character.isLetter(word.charAt(0)) || !Character.isDigit(word.charAt(word.length()-1)) || word.matches("[\\s\\S]*\\s[\\s\\S]*")) {
    return false;
} else {   
    return true;
}

The sonarqube says "Replace this if-then-else statement by a single return statement." I have no idea how to modify it.

Upvotes: 0

Views: 1037

Answers (3)

Pablo
Pablo

Reputation: 33

Timsmelik is on point here, and I agree with his answer. So I'm going to focus on how you could make this code more readable by adding a few private methods, maybe ending up with something like this:

public boolean isValid(String word) {
     return isNotNull(word) && hasAtLeastThreeChars(word) && isFirstCharALetter(word) && isLastCharADigit(word) && hasNoWhiteSpaces(word)
}

...essentially creating a private method for each validation you perform. This adds some lines of code, but it becomes crystal clear at first glance what's going on in that giant concatenation of conditions. I personally prefer readability over compact code.

However, since you're already using a regex to validate there are no whitespaces in the string, you could also expand that regex to validate some of the other points? You know that your string must have more than 2 chars, the first one must be a letter, the last one a digit, etc. If you modified your regex to also validate those points, you could save some lines. The only con of doing that would be that regexs can be quite obscure when you encounter one in code. So you'll probably hate yourself a little bit when you come up against that code in the future.

Hope this helped in any way

Upvotes: 3

yaloner
yaloner

Reputation: 758

Consider the following line of code:

return !(word == null || word.length() < 3 || !Character.isLetter(word.charAt(0)) || !Character.isDigit(word.charAt(word.length()-1)) || word.matches("[\\s\\S]*\\s[\\s\\S]*"));

The ! function as a NOT, and inverse the statement. All the rest should be familiar to you since you wrote it :)

Upvotes: 1

timsmelik
timsmelik

Reputation: 742

If you have something like this:

if (myObj == null) {
    return false;
} else {
    return true;
}

You can just replace the if-else with a return statement:

return myObj != null;

Note that since the return value was false if the expression in the if-statement was true, we have to inverse the return value.

Upvotes: 2

Related Questions