Nested if statements could be combined

[carlwilson]

There are 66 occurrences of this issue. Why is this an issue? Since: PMD 3.1

Sometimes two consecutive 'if' statements can be consolidated by separating their conditions with a boolean short-circuit operator.

Example(s):

void bar() {
    if (x) {            // original implementation
        if (y) {
            // do stuff
        }
    }
}

void bar() {
    if (x && y) {       // optimized implementation
        // do stuff
    }
}

Codacy's full list of occurrences can be found here

[nvanderperren]

Hi, I'm trying this one, but I don't really understand some code in the file jhove-core/src/main/java/edu/harvard/hul/ois/jhove/RFC1766Lang.java (codacy: line 68 has a nested if-statement that could by combined)

(starting from line 63)

if (ch == '-') {
    taglength = 0;

        // If this is the primary tag, do some checks
    if (ntags++ == 0) {
        if (taglength == 1) {  // taglength == 1, but in line 64 taglength = 0
        if (firstChar != 'i' && firstChar != 'x') {
        return false; // nested if -> returns false
            }
            else if (taglength != 2) { // ??? taglength == 1, so obviously taglength != 2
            return false; // -> also returns false
            }
        }
    }
}

[tledoux]

Hi @nvanderperren, you are correct in saying the code is wrong. According to RFC1766, the tag are define like

Language-Tag = Primary-tag *( "-" Subtag )
    Primary-tag = 1*8ALPHA
    Subtag = 1*8ALPHA

and the primary-tag is either 'i', 'x' or a 2 letters ISO639.

So the code should be

            if (ch == '-') {
                // If this is the primary tag, do some checks
                if (ntags++ == 0) {
                    if (taglength == 1) {
                        if (firstChar != 'i' && firstChar != 'x') {
                            return false;
                        }
                    } else if (taglength != 2) {
                            return false;
                    }
                }
                taglength = 0;

I suppose Unit tests should verify that this is correct or not...