8

Given a simple if statement in the form

public static String test(boolean flag) {
    if (!flag) {
        return "no";
    } else {
        System.out.println("flag enabled");
    }
    return "yes";
}

or

public static String test(final boolean flag) 
{
    if (flag) 
    {
        System.out.println("flag enabled");
        return "yes";
    } 
    else 
    {
        return "no";
    }
}

Eclipse gives me the warning, underlining the whole else block

Statement unnecessarily nested within else clause. The corresponding then clause does not complete normally

However, this...

public static String test(final boolean flag) 
{
    if (flag) 
    {
        System.out.println("flag enabled");
        return "yes";
    }
    return "no";
}

does not give the warning.

this question seems related, but I'm not using return in the else of the first example, and else is not finally.

I understand that this is simply a preference, and that it can be turned off. However, I don't like ignoring things simply because I don't understand what is the problem. And I can't seem to find any documentation on why Eclipse added this as a configurable warning.

So in all, What does The corresponding then clause does not complete normally mean? What is the problem that this warning is trying to protect me from?

Tezra
  • 8,463
  • 3
  • 31
  • 68
  • now this is a much better focused good question, and more likely a [duplicate](https://stackoverflow.com/questions/2681883) or [duplicate](https://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement) now. –  May 15 '18 at 17:12
  • @feelingunwelcome I was afraid to word like that in the first draft fearing it was 'too broad'. After pkpnd added examples to his answer, I realized that was exactly the question I was trying to ask, and it's not too broad. I would argue the other question is too broad, but is very closely related to this one. – Tezra May 15 '18 at 17:19
  • The later isn't a dup, as this warning is not about multiple returns. Eclipse is fine with that. It was the if-return-else that is the problem. – Tezra May 15 '18 at 17:20
  • I don't like any of the answers from [unnecessary 'else' statement](https://stackoverflow.com/questions/2681883/unnecessary-else-statement). The question was asked too broad, so it failed to answer why people would chose to avoid this pattern. (The practical root of it) – Tezra May 15 '18 at 17:27

1 Answers1

17

You can remove the else clause and that will remove the warning.

To answer your other question, "The corresponding then clause does not complete normally" is referring to the fact that the "then" clause has a return statement. Completing "normally" means control flow continues to the statements after the "else" block, but this doesn't happen since you returned.

There is no logical, functional, or runtime difference between:

// Gets warning
public static String test(final boolean flag) {
    if (flag) {
        System.out.println("flag enabled");
        return "yes";
    } else {
        return "no";
    }
}

and

// Doesn't get warning
public static String test(final boolean flag) {
    if (flag) {
        System.out.println("flag enabled");
        return "yes";
    }
    return "no";
}

So it boils down to a style preference. Why might Eclipse think this is worth a warning? Think of it as similar to "unreachable code" -- an aspect of the code is unnecessarily (and perhaps unusually) verbose, and this usually merits another look to ensure that no logic errors are hiding. If I wrote the first code snippet and got the warning, I'd be thankful for Eclipse and happily improve the code, arriving at the second snippet. But again, that's just because I prefer the second snippet.


Here's a scenario where this might come into play and affect functionality (unintentionally). Suppose the code originally looks like this:

// Returns true if the dog was fed
boolean feedDog(Dog dog) {
    if (!dog.isHungry()) {
        return false;
    }
    Food food;
    if (dog.isBadToday()) {
        food = getDefaultFood();
    } else {
        food = getTreat();
    }
    dog.feed(food);
    return true;
}

Sometime later, the team decides that feedDog should only return true if the dog was fed a treat. What if the code is modified to:

// Returns true if the dog was fed *a treat*
boolean feedDog(Dog dog) {
    if (!dog.isHungry()) {
        return false;
    }
    Food food;
    if (dog.isBadToday()) {
        food = getDefaultFood();
        return false; // Dog wasn't fed a treat
    } else {
        food = getTreat();
    }
    dog.feed(food);
    return true;
}

Now there's a bug because the programmer went with the shortest change that "looked right at first glance" -- the poor dog doesn't get fed if it misbehaved. Eclipse's warning is saving the dog here, telling the programmer to pay attention to the logic surrounding the if statement, which doesn't seem right anymore (according to Eclipse). Hopefully the programmer will heed the warning and avoid returning from the if block:

// Returns true if the dog was fed *a treat*
boolean feedDog(Dog dog) {
    if (!dog.isHungry()) {
        return false;
    }
    Food food;
    boolean fedTreat = false;
    if (dog.isBadToday()) {
        food = getDefaultFood();
    } else {
        food = getTreat();
        fedTreat = true;
    }
    dog.feed(food);
    return fedTreat;
}
Robert Harvey
  • 178,213
  • 47
  • 333
  • 501
k_ssb
  • 6,024
  • 23
  • 47