Does the last element in a loop deserve a separate treatment?
Asked Answered
U

13

20

When reviewing, I sometimes encounter this kind of loop:

i = begin
while ( i != end ) {    
   // ... do stuff
   if ( i == end-1 (the one-but-last element) ) {
      ... do other stuff
   }
   increment i
}

Then I ask the question: would you write this?

i = begin
mid = ( end - begin ) / 2 // (the middle element)
while ( i != end ) {    
   // ... do stuff
   if ( i > mid ) {
      ... do other stuff
   }
   increment i
}

In my opinion, this beats the intention of writing a loop: you loop because there is something common to be done for each of the elements. Using this construct, for some of the elements you do something different. So, I conclude, you need a separate loop for those elements:

i = begin
mid = ( end - begin ) / 2 //(the middle element)
while ( i != mid ) {    
   // ... do stuff
   increment i
}

while ( i != end ) {
   // ... do stuff
   // ... do other stuff
   increment i
}

Now I even saw a question on SO on how to write the if-clause in a nice way... And I got sad: something isn't right here.

Am I wrong? If so, what's so good about cluttering the loop body with special cases, which you are aware of upfront, at coding time?

Unsought answered 1/10, 2008 at 8:4 Comment(3)
All this is great. I can improve my personal work style with. ThxEstimable
"you loop because there is something common to be done for each of the elements." It depends how general that "something" is. It might be common just that all elements are cars. Some are blue, others are red. If blue then choose blue color, if red then choose red color to paint. So at a certain point, decisions in loops can make sense although. But at some point, it will become obvious you should split the loop.Estimable
@peter_the_oak: I agree that you make decisions based on values you encounter in the loop, but not based on the index.Unsought
P
7

@xtofl,

I agree with your concern.

Million times I encountered similar problem.

Either developer adds special handling for first or for last element.

In most cases it is worth to just loop from startIdx + 1 or to endIdx - 1 element or even split one long loop into multiple shorter loops.

In a very rare cases it's not possible to split loop.

In my opinion uncommon things should be handled outside of the loop whenever possible.

Pickerel answered 1/10, 2008 at 8:15 Comment(0)
P
23

I don't think this question should be answered by a principle (e.g. "in a loop, treat every element equally"). Instead, you can look at two factors to evaluate if an implementation is good or bad:

  1. Runtime effectivity - does the compiled code run fast, or would it be faster doing it differently?
  2. Code maintainability - Is it easy (for another developer) to understand what is happening here?

If it is faster and the code is more readable by doing everything in one loop, do it that way. If it is slower and less readable, do it another way.

If it is faster and less readably, or slower but more readable, find out which of the factors matters more in your specific case, and then decide how to loop (or not to loop).

Piroshki answered 1/10, 2008 at 8:58 Comment(0)
T
11

I know I've seen this when people tried to join elements of an array into a comma-seperated string:

for(i=0;i<elements.size;i++) {
   if (i>0) {
     string += ','
   }
   string += elements[i]
}

You either have that if clause in there, or you have to duplicate the string += line again at the end.

The obvious solution in this case is

string = elements.join(',')

But the join method does the same loop internally. And there isn't always a method to do what you want.

Tradesman answered 1/10, 2008 at 8:11 Comment(0)
P
7

@xtofl,

I agree with your concern.

Million times I encountered similar problem.

Either developer adds special handling for first or for last element.

In most cases it is worth to just loop from startIdx + 1 or to endIdx - 1 element or even split one long loop into multiple shorter loops.

In a very rare cases it's not possible to split loop.

In my opinion uncommon things should be handled outside of the loop whenever possible.

Pickerel answered 1/10, 2008 at 8:15 Comment(0)
A
6

I came to a realization that when I put special cases in a for loop, I'm usually being too clever for my own good.

Alsworth answered 1/10, 2008 at 8:12 Comment(0)
P
6

In the last snippet you posted, you are repeating code for // .... do stuff.

It makes sense of keeping 2 loops when you have completely different set of operations on a different set of indices.

i = begin
mid = ( end - begin ) / 2 //(the middle element)
while ( i != mid ) {    
   // ... do stuff
   increment i
}

while ( i != end ) {
   // ... do other stuff
   increment i
}

This not being the case, you would still want to keep one single loop. However fact remains that you still save ( end - begin ) / 2 number of comparisons. So it boils down to whether you want your code to look neat or you want to save some CPU cycles. Call is yours.

Pseudoscope answered 1/10, 2008 at 9:3 Comment(0)
A
5

I think you have it entirely nailed. Most people fall into the trap of including conditional branches in loops, when they could do them outside: which is simply faster.

For example:

if(items == null)
    return null;

StringBuilder result = new StringBuilder();
if(items.Length != 0)
{
    result.Append(items[0]); // Special case outside loop.
    for(int i = 1; i < items.Length; i++) // Note: we start at element one.
    {
        result.Append(";");
        result.Append(items[i]);
    }
}
return result.ToString();

And the middle case you described is just plain nasty. Imagine if that code grows and needs to be refactored into different methods.

Unless you are parsing XML <grin> loops should be kept as simple and concise as possible.

Amphibiotic answered 1/10, 2008 at 10:1 Comment(0)
U
3

I think you are right about the loop being meant to deal with all elements equally. Unfortunately sometimes there are special cases though and these should be dealt with inside the loop construct via if statements.

If there are lots of special cases though you should probably think about coming up with some way to deal with the two different sets of elements in separate constructs.

Undercoating answered 1/10, 2008 at 8:12 Comment(0)
C
3

I prefer to simply, exclude the element from the loop and give a spearate treatment outside the loop

For eg: Lets consider the case of EOF

i = begin
while ( i != end -1 ) {    
   // ... do stuff for element from begn to second last element
   increment i
}

if(given_array(end -1) != ''){
   // do stuff for the EOF element in the array
}
Coffman answered 6/4, 2013 at 5:55 Comment(0)
M
2

Of course, special-casing things in a loop which can be pulled out is silly. I wouldn't duplicate the do_stuff either though; I'd either put it in a function or a macro so I don't copy-paste code.

Millennial answered 1/10, 2008 at 8:8 Comment(0)
F
2

Which one performs better?

If the number of items is very large then I would always loop once, especially if you are going to perform some operation on every item. The cost of evaluating the conditional is likely to be less than looping twice.

Oops, of course you are not looping twice... In which case two loops is preferable. However, I maintain that the primary consideration should be performance. There's no need to incur the conditional in the loop (N times) if you can partition the work by a simple manipulation of the loop bounds (once).

Farthermost answered 1/10, 2008 at 8:10 Comment(1)
I never mentioned looping twice - I hinted to split the loop in two parts.Unsought
A
2

Another thing I hate to see is the for-case pattern:

for (i=0; i<5; i++)
{
  switch(i)
  {
    case 0:
      // something
      break;
    case 1:
      // something else
      break;
    // etc...
  }
}

I've seen this in real code.

Amoy answered 1/10, 2008 at 8:14 Comment(1)
If the cases just execute in order and have no common code, that would seem silly. If most iterations are handled by default or common code, however, and there are enough exceptions to favor the use of a switch rather than if tests, I would see nothing wrong with using a switch to handle scattered oddball cases (e.g. having a loop that counts from 1 to 255 and does something weird for index values of 7, 8, 9, 10, 12, 13, and 27 may seem icky, but sometimes programs have to model things which are icky in the real world).Moorland
J
1

The special case should be done outside the loop if it is only to be performed once.

However, there may be an index or some other variable(s) that are just easier to keep inside the loop due to scoping. There may also be a contextual reason for keeping all the operations on the datastructure together inside the loop control structure, though I think that is a weak argument on its own.

Jaehne answered 1/10, 2008 at 9:6 Comment(0)
I
1

Its just about using it as per need and convenience. There is as such no mentions to treat elements equally and there is certainly no harm clubbing the features which language provides.

Idiomatic answered 1/10, 2008 at 9:10 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.