Good Practice: Loop And If statement [closed]

Question

(I'm copy/pasting the same question I posted on Codereview here: https://codereview.stackexchange.com/questions/1747/good-practice-loop-and-if-statement)

I'd like to know what is the best practice:

version A:

loop1
  if condition1
    code1

  if condition2
    code2

  if condition3
    code3

Or, version B:

if condition1
  loop1 with code1

if condition2
  loop1 with code2

if condition3
  loop1 with code3

I've implemented version B cause it's more readable for me, and always checking the same condition seems absurd.

But looping n times the same array could be seen as absurd too :)

Answer 1

From the fact that you are considering version B, which has the conditions outside of the loops, I assume that the the truth value of the conditions do not vary with the elements within the array. Otherwise, your question will not make sense.

Conclusion

If the conditions are complicated so that they take much time in evaluation relative to the size of the array, then version B is faster. If it is the other way around, then version A is faster.

And which is faster should coincide in this case with which strategy you should take because it is more comprehensible to have a simple structure embedded within a complicated structure rather than having an embedding of a complicated structure within a simple structure.

Explanation

When the condition is simple enough relative to the size of the array, the cost of iteration overweighs the cost of evaluating the condition. As observed below with ruby, version B is slower.

$a = (1..10000)

def versionA
  $a.each do
    nil if true
    nil if false
    nil if true
  end
end

def versionB
  $a.each {nil} if true
  $a.each {nil} if false
  $a.each {nil} if true
end

require 'benchmark'
n = 10000
Benchmark.bmbm do|b|
  b.report('A'){n.times{versionA}}
  b.report('B'){n.times{versionB}}
end

Rehearsal -------------------------------------
A   7.270000   0.010000   7.280000 (  7.277896)
B  13.510000   0.010000  13.520000 ( 13.515172)
--------------------------- total: 20.800000sec

        user     system      total        real
A   7.200000   0.020000   7.220000 (  7.219590)
B  13.580000   0.000000  13.580000 ( 13.605983)

On the other hand, if evaluating the conditions is more costly relative to iteration over the array, then the effect of the former will become more crucial than the latter, and the speed will be the other way around.

$a = (1..100)

def versionA
  $a.each do
    nil if (1..10).each{nil} && true
    nil if (1..10).each{nil} && false
    nil if (1..10).each{nil} && true
  end
end

def versionB
  $a.each {nil} if (1..10).each{nil} && true
  $a.each {nil} if (1..10).each{nil} && false
  $a.each {nil} if (1..10).each{nil} && true
end

require 'benchmark'
n = 10000
Benchmark.bmbm do|b|
  b.report('A'){n.times{versionA}}
  b.report('B'){n.times{versionB}}
end

Rehearsal -------------------------------------
A   2.860000   0.000000   2.860000 (  2.862344)
B   0.160000   0.000000   0.160000 (  0.169304)
---------------------------- total: 3.020000sec

        user     system      total        real
A   2.830000   0.000000   2.830000 (  2.826170)
B   0.170000   0.000000   0.170000 (  0.168738)

Answer 2

Version B is much more appreciable, since you may generally desire to

 if condition1
     loop1 with code1
 if condition2
     loop2 with code2
 if condition3
     loop3 with code3

which can't be fast-and-easy refactored from A.

Edit: however, if the loop "condition" is dynamically driven by it's body, you should use A. More info on the code semantics is needed.

Answer 3

Well, I think the answer is more qualified than it would seem. Most coding practices suggest keeping code blocks small, so from that angle, it really comes down to "how big is the entire block"? But I would consider this according to intent of the code. For example:

foreach (widgets)
    if (widget is red) put in left bin
    if (widget is blue) put in center bin
    if (widget is green) put in right bin

vs:

if (making widgets red) 
    foreach (widgets) put in left bin
if (making widgets blue)
    foreach (widgets) put in center bin
if (making widgets green) 
    foreach (widgets) put in right bin

Each structure is telling a different story about your intent. In the first case, we are iterating over widgets and making decisions on each, while in the latter case we're iterating over decisions in an unrolled loop and making changes to the widgets. This of course makes more sense for else-if cases, but in general, if you can make a statement about a block of code easily (eg, in this loop, I sort widgets by color) without having to write a paragraph, it's the most understandable code, and that's the goal of best practices to begin with.

Of course, performance is also an issue. I guess performance and best practices are uneasy friends. If you're hitting the DB for the iteration to get each row, or if the iteration is over thousands of entries, it's sometimes necessary to write less-intent-oriented code that reduces the slowest action to a minimum. It's not the first consideration, but writing efficient code requires one to look at things from all the angles and make compromises, so the rules are not so hard and fast.

Answer 4

In scenario A, you're making three if-checks for every reiteration of the loop. In scenario B, you only have three if-checks total. In terms of time complexity, the second version is much better.