[Gerry Kan]

Howdy Folks:

I have a block of code in a do loop that is abbreviated as follows:

integer :: k0, k1, k, kN

k0 = 0
kN = 100
k1 = k0 + 1

do k = k1,kN
   ! do something
end do

Some guy tells me that the do loop has to be done like this:

do k = k0+1,kN

Is there any hidden reason that makes the second approach superior? Also, wouldn't k0+1 run a risk of being needlessly evaluated in every iteration of the do loop?

Your comments would be quite welcomed.

Gerry.

[FMDenaro]

that is strange ...

[flotus1]

Are you sure your example retains all of the original context if this issue?
Did "that guy" give any reason for his recommendation?

[Gerry Kan]

Howdy:

This is also very strange to me. "This guy" is the code reviewer, so my first reaction was, WTF. All he said was something along the line of do this or I don't get his blessings.

Having said that the example is the absolute minimum that I can contain within a single block. To be more specific, k0 and kN are global variables already defined in another module. The variable k1 (and counter k) are locally declared.

I don't know if this would affect your perception for this. If k0 were declared as a parameter then the compiler would be smart enough to calculate k0+1 before the do loop. However k0 is not a parameter so what the compiler would do is kind of up in the air. Calculating k0+1 beforehand should make this behavior more clear, I would think.

Whadayathink, Gerry.

[sbaffini]

I can see few reasons why, but honestly none of them is reasonably linked to performances, given the piece of code we have seen.

First of all, as there is an official reviewer for the code, I also expect code styles to be in place (despite the horrible decision of having a global variable... and naming it k0!!!!), so you might be simply using k0 in a way that doesn't conform to the rest of the code.

A second reason might be linked to be able to catch meaningful pieces of code when searching for k0. Say, a grep from command line, would return, in your case, an obscure line and you would have no idea what's going on without looking at the whole source code.

Finally, it may sound silly, but you are actually using one more line of code than is actually needed. Loose control on this and you end up with thousands more lines of code in a project.

However, if there is a performance issue (which we can't spot from here), then I bet my soul that this is not the actual solution to the actual problem

[Gerry Kan]

Thanks Paolo.

Of course, these global variables wouldn't be called k0 and kN in the actual code. I renamed them here for illustration, and also to protect my identity from the few of those who might know the code.

And if we have to code accordingly to its greppability we will end up something that look quite ugly in no time.

However I might have to checck their punch card era coding standard to see if it has to be done like that. Last time I checked there was no such entry but you'll never know.

However, if there is a performance issue (which we can't spot from here), then I bet my soul that this is not the actual solution to the actual problem

I don't quite know what you meant by that. Do you mean I shouldn't have defined k1 to pre-calulate k0+1 in the first place?

Gerry.

[sbaffini]

Originally Posted by Gerry Kan

Thanks Paolo.

Of course, these global variables wouldn't be called k0 and kN in the actual code. I renamed them here for illustration, and also to protect my identity from the few of those who might know the code.

And if we have to code accordingly to its greppability we will end up something that look quite ugly in no time.

However I might have to checck their punch card era coding standard to see if it has to be done like that. Last time I checked there was no such entry but you'll never know.



I don't quite know what you meant by that. Do you mean I shouldn't have defined k1 to pre-calulate k0+1 in the first place?

Gerry.

No, I simply meant that if, by any chance, your code was corrected for performance reasons, it is a mistake on their side. Even if, literally, your code causes indeed troubles, it is a larger issue to be looked elsewhere, certainly not a single line such this one.

[piu58]

I k1 is not needed elsewhere it is useless to have a variable only for the purpose of the loop. I find the second version more clear and better readable. I don't believe that is faster or less memory consuming.

[Gerry Kan]

So it basically comes down to that he is used to writing do loops this way, and I the other way. Correct? Again I need to check the standards to see whether it is codified.

Gerry.

[Gerry Kan]

Originally Posted by sbaffini

First of all, as there is an official reviewer for the code, I also expect code styles to be in place.

Okay, I checked their coding standards, and there was no mention of needing to use k0+1 in the do loop.

Yay!

[sbaffini]

Originally Posted by Gerry Kan

Okay, I checked their coding standards, and there was no mention of needing to use k0+1 in the do loop.

Yay!

I was more like referencing to the fact that, maybe, there are several such loops in the code, and they are all written the same way. Yet, I would expect to be explicitly told so if there isn't an official document mentioning it.

So, if it isn't so, I honestly have no idea why it might be so important at all.

[Gerry Kan]

Originally Posted by sbaffini

So, if it isn't so, I honestly have no idea why it might be so important at all.

Paolo:

I got an inkling that he just wanted to show me how much he disliked my approach because he disagreed with it from the get go. He left five full pages of comments and cc'd his boss, who has the last word, even stressing that the whole thing be rewritten to his approach. But of all his criticisms, this one was ironically the most difficult to defend.

Gerry.

[Simbelmynë]

From the number of replies here (with guesses) it seems that the only way you might get "closure" in this case is to actually ask the code reviewer about this. It is either this or just perform the changes and leave it.


By the way, if you have several loops beginning with k1 then I think it is more clean to write it like you did. Similar to using kN. If it is only one loop then I guess kN would be bad to use as well if this was some "save space" argument so this may not be the reason.

[Gerry Kan]

Originally Posted by Simbelmynë

By the way, if you have several loops beginning with k1 then I think it is more clean to write it like you did. Similar to using kN. If it is only one loop then I guess kN would be bad to use as well if this was some "save space" argument so this may not be the reason.

I was taught you should never introduce expressions of any kind in the loop bound as you don't know whether the limits will be calculated beforehand or are evaluated at each iteration. In this case the impact is not so heavy, since you are only adding 1, and the bounds (supposedly) don't change throughout the loop. But to me this is still a bad habit.

If one wants to work with floating bounds, a while loop or internal break mechanisms would be more effective.

Gerry.

[Simbelmynë]

I do not have sufficient coding, or rather compiler, experience to say anything about this. I have a hard time believing that this has any measurable impact on performance. It should be fairly easy to benchmark though.



I was just pointing out that the reviewer is inconsistent if the reason would have been "to save space". However, this is probably not the reason so I think you can disregard my last point