If statements inside or outside loops?

Question

Is it better if I do this:

foreach my $item ( @array ) {
   if ( $bool ) {
     .. code ..
   }
   else {
     .. code ..
   }
}

or

if ( $bool ) {
   foreach my $item ( @array ) {
   }
}
else {
   foreach my $item ( @array ) {
   }
}

Answer 1

I suggest you time both and see for yourself, but I don't expect the difference to be huge.

Answer 2

I would leave premature optimization aside.

"Premature optimization is the root of all evil" - Donald Knuth

You should go for maintainability first and foremost. Group them in the way that makes more sense taking into account the logical structure of the code (such as grouping related statements together).

If you later determine that performance is an issue, try measuring with something like a profiler to see where the bottlenecks are. Chances are, it's not there. From Code Complete 2:

Barry Boehm reports that 20 percent of a program's routines consume 80 percent of its execution time. In his classic paper "An Empirical Study of Fortran Programs," Donald Knuth found that less than four percent of a program usually accounts for more than 50 percent of its run time.

We shouldn't try to guess where to optimize before it is necessary since most of us are really bad at guessing where that slow portion of our code is. Programmers who optimize as they go also spend about 96% of their time optimizing code that doesn't need to be optimized. Another thing to take into account is that code tuning (as in this example) considers a tradeoff between readability and maintainability for performance. Again, from Code Complete 2:

Focusing on optimization during initial development detracts from achieving other program objectives. Developers immerse themselves in algorithm analysis and arcane debates that in the end don't contribute much value to the user. Concerns such as correctness, information hiding, and readability become secondary goals, even though performance is easier to improve later than these other concerns are. Post hoc performance work typically affects less than five percent of a program's code. Would you rather go back and do performance work on five percent of the code or readability work on 100 percent?

I'm not saying don't optimize, but optimize code only in the end, when you have the luxury of the big picture and tools to point you in the right direction.

EXTRA: To answer the question of performance itself, though, I will yet again refer to Code Complete:

This ["unswitching" the code] is good for about a 20 percent time savings:

Language        Straight Time    Code-Tuned Time    Time Savings
C++             2.81             2.27               19%     
Java            3.97             3.12               21%
Visual Basic    2.78             2.77               <1%
Python          8.14             5.87               28%

A hazard distinct to this case is that the two loops have to be maintained in parallel. [...] you have to remember changing the code in both places, which is an annoyance for you and a maintenance headache for anyone else who has to work with the code.

This example also illustrates a key challenge in code tuning: the effect of any specific code tuning is not predictable. The code tuning produced significant improvements in three of the four languages but not in Visual Basic. To perform this specific optimization in this specific version of Visual Basic would produce less maintainable code without any offsetting gain in performance. The general lesson is that you must measure the effect of each specific optimization to be sure of its effect - no exceptions.

Check this other question here on SO. And this from the first edition of Code Complete.

Answer 3

If you're optimizing for speed, the second (foreach loops inside the if branches) should be faster, since you won't be doing the test in each loop iteration.

Answer 4

The second will be faster since many fewer comparisons. -- The compare is outside the loop rather than inside.

And since the comparison variable is a loop invariant, I'd be surprised if it wasn't also clearer coding.

Actual speed difference (wall clock time) depends on size of the array

Answer 5

Simply evaluating a boolean variable as you have done here, these are roughly equivalent. However, if the variable were replaced with a complicated expression that took a long time to evaluate, the second example would be better because it would only be evaluated once.

Answer 6

Everyone seems stuck on the performance issue.

It's almost always better to never have to repeat code. That is, typing the same thing more than once should be painful to you. Since you haven't said anything about the code in each, I'll assume that you want to do different things in each case. My preference is to separate the details of the iteration from the particular processing.

 my $sub_ref = $bool ? make_true_function() : make_false_function();

 foreach my $element ( @array ) {
      $sub_ref->( $element );
      }

 sub make_true_function  { sub { ... } }
 sub make_false_function { sub { ... } }

That might lose a tiny bit in performance, but it's a lot easier to look at because it's less tangled code. The foreach doesn't care anything about branching or how you made your decision. This works nicely when you want to have more branches too. As long as the right thing shows up in $sub_ref, you don't change the iteration code.