Plane: Change a return statement that is not executed

[muramura]

When AP_SCRIPTING_ENABLED is enabled, there are RETURN statements that are not executed.
I would make sure to select the RETURN statement

[magicrub]

Lgtm

[peterbarker]

I do not like the change introduced here. For the same reason we omit else on if statements which always return, we should omit the #else here.

Here's the early return pattern we use:

int foo() {
    if (bar()) {
        return 17;
    }
    if (baz()) {
       return 42;
    }
    return 3;
}

This is what it looks like in preprocessor:

int foo() {
#if FOO
    return BOB;
#endif

#if BAR
     return BAX;
#endif

    return XYZZY; 

See how that is symmetric?

We've rejected a lot of "pointless else" patches, we should not be adding them, they just make things more difficult to understand at a glance.

[magicrub]

@peterbarker I don't understand your point. The # else looks fine and is appropriate here.

[peterbarker]

@peterbarker I don't understand your point. The # else looks fine and is appropriate here.

Would you think this is correct? I certainly don't.

bool myfunction()
{
#if AP_OPTION_A
    if (option_a_is_looking_bad()) {
        GCS_SEND_TEXT(MAV_SEVERITY_ERROR, "Everything is looking fine");
        return false;
    }
#endif

    GCS_SEND_TEXT(MAV_SEVERITY_WARNING, "Continuing world domination");
    if (frob_the_widget()) {
        GCS_SEND_TEXT(MAV_SEVERITY_WARNING, "The master plan is underway");
        return true;
    }

#if AP_OPTION_B
    if (coax_the_sylvanberry()) {
        GCS_SEND_TEXT(MAV_SEVERITY_CRITICAL, "Sylvanberry engaged");
        return true;
    }
#else
    return false;
#endif
}

.... because its natural conclusion is this:

bool myfunction()
{
#if AP_OPTION_A
    if (option_a_is_looking_bad()) {
        GCS_SEND_TEXT(MAV_SEVERITY_ERROR, "Everything is looking fine");
        return false;
    }
#else
    GCS_SEND_TEXT(MAV_SEVERITY_WARNING, "Continuing world domination");
    if (frob_the_widget()) {
        GCS_SEND_TEXT(MAV_SEVERITY_WARNING, "The master plan is underway");
        return true;
    }
#if AP_OPTION_B
    if (coax_the_sylvanberry()) {
        GCS_SEND_TEXT(MAV_SEVERITY_CRITICAL, "Sylvanberry engaged");
        return true;
    }
    return false;
#endif
    return false;
#endif
}

[peterbarker]

Closing as contrary to our preferred early-return pattern.