3rgo
3rgo

Reputation: 3153

What is better: multiple "if" statements or one "if" with multiple conditions?

For my work I have to develop a small Java application that parses very large XML files (~300k lines) to select very specific data (using Pattern), so I'm trying to optimize it a little. I was wondering what was better between these 2 snippets:

if (boolean_condition && matcher.find(string)) {
    ...
}

OR

if (boolean_condition) {
    if (matcher.find(string)) {
        ...
    }
}

Other details:

Thanks for your help.

Upvotes: 68

Views: 113762

Answers (10)

TTDS
TTDS

Reputation: 104

Most would prefer to use the below one, because of "&&".

if (boolean_condition && matcher.find(string)) {
...
}

We normally called these as "short-circuit (or minimum evaluation)". It means the 2nd argument (here it is "matcher.find(string)") is only evaluated only if the 1st argument doesn't have sufficient information to determine the value of the expression. As an example, if the "boolean_condition" is false, then the overall condition must be false (because of here logical AND operator). Then compiler won't check the 2nd argument which will cause to reduce the running time of your code.

Upvotes: 0

55 Cancri
55 Cancri

Reputation: 1195

I recommend extracting your expression to a semantically meaningful variable and then passing that to your evaluation. Instead of:

if (boolean_condition && matcher.find(string)) { ... }

Assign the expression to a variable, then evaluate the variable:

const hasItem = boolean_condition && matcher.find(string)

if (hasItem) { ... }

With this method, you can keep even the most complex evaluations readable:

const hasItem = boolean_condition && matcher.find(string)

const hasOtherThing = boolean_condition || boolean_condition

const isBeforeToday = new Date(string) < new Date()

if (hasItem && hasOtherThing && isBeforeToday) { ... }

Upvotes: 4

adarshr
adarshr

Reputation: 62573

One golden rule I follow is to "Avoid Nesting" as much as I can. But if it is at the cost of making my single if condition too complex, I don't mind nesting it out.

Besides you're using the short-circuit && operator. So if the boolean is false, it won't even try matching!

So,

if (boolean_condition && matcher.find(string)) {
    ...
}

is the way to go!

Upvotes: 76

Petter Friberg
Petter Friberg

Reputation: 21710

If you like to be compliant to Sonar rule squid:S1066 you should collapse if statements to avoid warning since it states:

Collapsible "if" statements should be merged

Upvotes: 3

sm14
sm14

Reputation: 119

I tend to see too many && and || strung together into a logic soup and are often the source of subtle bugs.

It is too easy to just add another && or || to what you think is the right spot and break existing logic.

Because of this as a general rule i try not to use either of them to avoid the temptation of adding more as requirements change.

Upvotes: 3

Mike Dunlavey
Mike Dunlavey

Reputation: 40649

In terms of performance, they're the same.

  • But even if they weren't

what's almost certain to dominate the time in this code is matcher.find(string) because it's a function call.

Upvotes: 1

Jonathan
Jonathan

Reputation: 3253

The following two methods:

public void oneIf(boolean a, boolean b)
{
    if (a && b)
    {   
    }
}

public void twoIfs(boolean a, boolean b)
{
    if (a)
    {
        if (b)
        {       
        }
    }
}

produce the exact same byte code for the method body so there won't be any performance difference meaning it is purely a stylistic matter which you use (personally I prefer the first style).

Upvotes: 24

Farrell
Farrell

Reputation: 508

Java uses short-circuiting for those boolean operators, so both variations are functionally identical. Therefore, if the boolean_condition is false, it will not continue on to the matching

Ultimately, it comes down to which you find easier to read and debug, but deep nesting can become unwieldy if you end up with a massive amount of braces at the end

One way you can improve the readability, should the condition become longer is to simply split it onto multiple lines:

if(boolean_condition &&
   matcher.find(string))
{
    ...
}

The only choice at that point is whether to put the && and || at the end of the previous line, or the start of the current.

Upvotes: 3

JB Nizet
JB Nizet

Reputation: 691635

Both ways are OK, and the second condition won't be tested if the first one is false.

Use the one that makes the code the more readable and understandable. For just two conditions, the first way is more logical and readable. It might not be the case anymore with 5 or 6 conditions linked with &&, || and !.

Upvotes: 7

Robby Pond
Robby Pond

Reputation: 73484

The first one. I try to avoid if nesting like that, i think it's poor style/ugly code and the && will shortcircuit and only test with matcher.find() if the boolean is true.

Upvotes: 2

Related Questions