Question

Long winding if conditions should be avoided if at all possible, yet sometimes we all end up writing them. Even if it's a very simple condition, the involved statements are sometimes simply very wordy, so the whole condition ends up being very lengthy. What's the most readable way to format those?

if (FoobarBaz::quxQuux(corge, grault) || !garply(waldo) || fred(plugh) !== xyzzy) {
    thud();
}

or

if (
    FoobarBaz::quxQuux(corge, grault)
 || !garply(waldo)
 || fred(plugh) !== xyzzy
) {
    thud();
}

or

if (FoobarBaz::quxQuux(corge, grault)
    || !garply(waldo)
    || fred(plugh) !== xyzzy) {
    thud();
}

or

thudable = FoobarBaz::quxQuux(corge, grault);
thudable ||= !garply(waldo);
thudable ||= fred(plugh) !== xyzzy;

if (thudable) {
    thud();
}

or any other preferences?

Was it helpful?

Solution

Often, a long if condition is the sign of code that needs refactoring, but sometimes you can't avoid it. In those cases, I prefer the first:

if (bar || baz || quux) { ... }

Because you're able to tell what's going on with one line. However, I'd much rather do something like this, when possible:

function foo() {
  return bar || baz || quux;
}

if (foo()) { ... }

OTHER TIPS

I like keeping the operators at the end to indicate continuation:

if (the_function_being_called() != RETURNCODE_SUCCESS &&
    the_possibly_useful_recovery_strategy() == RETURNCODE_EPICFAIL &&
    this_user_has_elected_to_recieve_error_reports)
{
    report_error();
}

I am a big fan of meaningful variable names:

const bool isInAStrangeCondition =
    FoobarBaz::quxQuux(corge, grault) ||
    !garply(waldo) ||
    fred(plugh) !== xyzzy;

if (isInAStrangeCondition) {
    thud();
}

Or refactor as a function, as mentioned above.

I break out the messier subexpressions, or all of them, as bool variables. Then the top-level boolean logic of the 'if' statement can be made clear. In the kind of work I do, it's not always several things ORed or ANDed.

bool goodblah = some_mess < whatever;
bool frobnacious = messy_crud != junky_expression;
bool yetanother = long_winded_condition;

if (goodblah || (frobnacious && yetanother))   {
    ...
}

This is especially good in a debugger, where I can look at all the bools before executing the 'if'.

I tend to align the operators at the start of new lines so I remember how I'm combining terms (both for long logic and long arithmetic). Like this:

if (first_attempt(data) == SUCCESS
    || (reusable(data) && second_attempt(data) == SUCCESS)
    || (still_reusable(data) && third_attempt(data) == SUCCESS))
  return SUCCESS;

This only works if I indent by 2-spaces or set set my environment to indent multiline predicates more, or else it would be hard to tell where the predicate ends and useful code begins.

I'm a fan of the following:

if (really_long_expression && another_really_really_long_expression && 
            another_very_long_expression_OMG_id_it_long){
    bugs();
}

This way it still looks like an if expression and not a broken-down-to-pieces if expression. The indentation helps in showing that it is a continuation of the previous line.

You can also indent it until the opening bracket is at the end of the previous line so that it's at the end of the if expression as it's supposed to be.

Licensed under: CC-BY-SA with attribution
scroll top