Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question

closed as primarily opinion-based by MichaelT, gnat, Doc Brown, Kilian Foth, jmo21 Oct 2 '13 at 10:37

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise.If this question can be reworded to fit the rules in the help center, please edit the question.

6 Answers 6

up vote 17 down vote accepted

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()) { ... }
share|improve this answer
2  
scrolling to the side vs vertical is not nearly the limitation it was in the bad old days... –  Bill Sep 14 '10 at 5:48
1  
and give a meaningfull (business) name to the function so that people understand what's tested here. –  Matthieu M. Oct 14 '10 at 17:46

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();
}
share|improve this answer
    
I think I like this one. I employ a lot of parenthesis to ensure I can understand the order of precedence too. –  Jasarien Sep 14 '10 at 13:08
1  
I prefer putting the logical operators on the start of the line so as I read a line I can easily see it is part of the conditional and not just a regular line of code. –  Snowman Apr 25 at 18:02

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.

share|improve this answer

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'.

share|improve this answer
    
I like this as well, but you do lose one benefit: it is no longer possible to short-circuit expensive comparisons anymore. –  Snowman Apr 25 at 18:03

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.

share|improve this answer

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.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.