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?
Another thing I hate to see is the for-case pattern:
I've seen this in real code.
I know I've seen this when people tried to join elements of an array into a comma-seperated string:
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
But the join method does the same loop internally. And there isn't always a method to do what you want.
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.
@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.
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.
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.
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:
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).