Is correct to assign to a variable using && with the same variable in Java?

Viewed 184

I'm making some changes into an old code than requires maintenance, and I found something like this

public boolean aMethod(){
    boolean isValid = true;

    **isValid = isValid &&** executeGetDocs(id.toString(),program().toString(), document.get(name));

  return isValid;
}

The method executeGetDocs may return true or false.

But I'm not sure if: **isValid = isValid && executeGetDocs ** is correct or is just redundant.

So here I am, can anyone please, give me some explanation about this?

Thanks a lot!

EDITED

Whole method:

private boolean validateDocs(List<Map<String, Object>> documentsList) {
        boolean isValid = true;
        StringBuilder gradoAntComp = new StringBuilder();
        for (Map<String, Object> document : documentsList) {
            String id = document.get("ID_DOC").toString();
            if (id.equalsIgnoreCase("25")) {
                isValid = getData(id.toString(),program().toString(), document.get(name))
            } else if (id.equalsIgnoreCase("26") ) {
                isValid = isValid && getDocs(id.toString(),program().toString(), document.get(name));
            } 
        }
        return isValid;
    }
3 Answers

Not that we see your entire method, we can see that you're attempting to assign to isValid within a loop. Written this way, the code is using the && conditional-and operator to only execute the method getDocs if isValid is still true.

If a previous assignment to isValid set it to false, then getDocs won't even be called; it will be short-circuited. This means that the && operator already knows that the result of the expression will be false, because isValid is false, so it "short-circuits" the evaluation and doesn't evaluate the other operand -- the call to getDocs.

This makes a lot of sense especially if getDocs is a method with a large overhead cost.

It was redundant in your original question. The original method you posted could be reduced to:

public boolean aMethod() {
    return executeGetDocs(id.toString(),program().toString(), document.get(name));
}

since (true && x) == x for any boolean x.

However, since you changed the question, the answer changes as well. Now the boolean variable may change multiple times within a loop. Therefore it makes sense to AND the previous value of the variable (which may be true or false) with another boolean, which might cause its value to change.

private boolean validateDocs(List<Map<String, Object>> documentsList) {
    boolean isValid = true;
    StringBuilder gradoAntComp = new StringBuilder();
    for (Map<String, Object> document : documentsList) {
        String id = document.get("ID_DOC").toString();
        if (id.equalsIgnoreCase("25")) {
            // this can set isValid to either false or true
            isValid = getData(id.toString(),program().toString(), document.get(name))
        } else if (id.equalsIgnoreCase("26") ) {
            // therefore performing logical AND between the previous value of
            // isValid and some additional boolean is meaningful - isValid will
            // be assigned true only if it was true before this statement AND
            // getDocs(id.toString(),program().toString(), document.get(name)) is true
            isValid = isValid && getDocs(id.toString(),program().toString(), document.get(name));
        } 
    }
    return isValid;
}

As you initialize isValid as true it's useless you can rewrite it as:

public boolean aMethod(){
    return executeGetDocs(id.toString(),program().toString(), document.get(name));
}
Related