Style Question (slightly O.T.) 
Author Message
 Style Question (slightly O.T.)

I'm reviewing some code provided by a vendor.  My first reaction is to
vomit all over it.  Before I use my fire axe, I need your collective
opinion.  What would you say if you consistently came across the
following code in a for loop?

        int i, n;

        /* n is set to some valid value */
        /* and then later we see */

        for (i=0; i<=n-1; i++)

my concern is the <= test on  i vs. n-1 instead of the normal (IMO)

        for (i=0; i<n; i++)

--
Jim Dalsimer



Sat, 08 Mar 2003 10:08:14 GMT  
 Style Question (slightly O.T.)

Quote:

>I'm reviewing some code provided by a vendor.  My first reaction is to
>vomit all over it.  Before I use my fire axe, I need your collective
>opinion.  What would you say if you consistently came across the
>following code in a for loop?

>    int i, n;

>    /* n is set to some valid value */
>    /* and then later we see */

>    for (i=0; i<=n-1; i++)

>my concern is the <= test on  i vs. n-1 instead of the normal (IMO)

>    for (i=0; i<n; i++)

I think that a case can be made for the clarity of i <= n - 1 since people tend
to think in terms of closed bounds. The loop variable takes on the
values from 0 to n-1, inclusive.  (In some other programming language, you
might even have a for loop construct like   for i = 0 to n - 1. Perhaps these
programmers were used to such a programming language). Also consider the
mahematical notation for summation:

        n-1
        ---
        \
        /    expr(i)
        ---
       i = 0

So don't be too {*filter*} these guys!  However the < n test is a nearly
ubiquitous C idiom; ignoring such an idiom does seem to be some kind of
indication of a lack of C cultural awareness perhaps stemming from inexperience
or from programming in isolation.

The value n is often the expression which yields the size of the array in terms
of the number of elements. Programmers are used to reading it and writing it,
and it's terser by three tokens.

--
Any hyperlinks appearing in this article were inserted by the unscrupulous
operators of a Usenet-to-web gateway, without obtaining the proper permission
of the author, who does not endorse any of the linked-to products or services.



Sat, 08 Mar 2003 10:29:43 GMT  
 Style Question (slightly O.T.)


Quote:
> I'm reviewing some code provided by a vendor.  My first reaction
> is to vomit all over it.  Before I use my fire axe, I need your
> collective opinion.  What would you say if you consistently came
> across the following code in a for loop?

>    int i, n;

>    /* n is set to some valid value */
>    /* and then later we see */

>    for (i=0; i<=n-1; i++)

> my concern is the <= test on  i vs. n-1 instead of the normal (IMO)

>    for (i=0; i<n; i++)

I would say nothing.  It would be terrible if the two styles are
freely mixed, because it unnecessarily slows down the reader.
It's also possible* that mixing the two results in more error-
prone code.

However, if "<= n-1" is used consistently, I can't say that it
would upset my stomach.  In fact, if this is the worst "offense"
you can cite, I don't think the code is all that terrible.  I
certainly would not suggest that you rewrite them, especially if
the code has undergone any testing.  In fact, I would suggest
that you follow their style when modifying their modules.

It's not about ego.  It's about avoiding the risk of introducing
bugs into tested software (for what gain?) and the possible risk
of introducing inconsistent code.

*  It's my theory that programmers expecting only one form of
   such code can spot off-by-one errors (like <= instead of <)
   more readily than programmers who must expect both forms.

Sent via Deja.com http://www.deja.com/
Before you buy.



Sat, 08 Mar 2003 10:55:58 GMT  
 Style Question (slightly O.T.)

Quote:
> So don't be too {*filter*} these guys!  However the < n test is a nearly
> ubiquitous C idiom; ignoring such an idiom does seem to be some kind of
> indication of a lack of C cultural awareness perhaps stemming from inexperience
> or from programming in isolation.

This is probably one of those "religious" style issues.

I've always thought about the difference between "<" and "<=" as
follows:

< (from 0 to some n) is useful to understand how many times the
loop will be run, as in (for i = 0;  i < 4; ++i) the loop will run
4 times... This is IMHO a better, more universal way to present things.

<= (from 0 to some n-1) is useful to describe the bounds of the
array on which the loop will operate. I like this technique much less,
because you can't say how many times it runs in a glance (it takes
your brain another few particles of a second to calculate n+1 ;)

However, I've seen enough code using both techniques.



Sat, 08 Mar 2003 15:15:10 GMT  
 Style Question (slightly O.T.)

Quote:
> I'm reviewing some code provided by a vendor.  My first reaction is to
> vomit all over it.  Before I use my fire axe, I need your collective
> opinion.  What would you say if you consistently came across the
> following code in a for loop?

> int i, n;

> /* n is set to some valid value */
> /* and then later we see */

> for (i=0; i<=n-1; i++)

> my concern is the <= test on  i vs. n-1 instead of the normal (IMO)

> for (i=0; i<n; i++)

It's a style issue, i tend to use the second one, but sometimes i use the first
one too. It depends on how you think on  the program the moment you write the
code.

Ioannis



Sat, 08 Mar 2003 03:00:00 GMT  
 Style Question (slightly O.T.)

Quote:

> Date: Tue, 19 Sep 2000 02:08:14 GMT

> Newsgroups: comp.lang.c
> Subject: Style Question (slightly O.T.)

> I'm reviewing some code provided by a vendor.  My first reaction is to
> vomit all over it.  Before I use my fire axe, I need your collective
> opinion.  What would you say if you consistently came across the
> following code in a for loop?

>    int i, n;

>    /* n is set to some valid value */
>    /* and then later we see */

>    for (i=0; i<=n-1; i++)

> my concern is the <= test on  i vs. n-1 instead of the normal (IMO)

>    for (i=0; i<n; i++)

IMO big time.

It depends on the use. For example.

If I have an enum of colors

red, green, amber, blinkingred, blinkinggreen, blinkingamber, off

and I want to do something for the cases when the color is blinking
I would use
if (color >=blinkingred&& color <=blinkingamber)
   do something
rather than
if (color >amber && color <off)
   do something
or worse
if (color >amber && color <=blinkingamber)
   do something
Identical logic, but one is preferable because it is more logical for
a reader to see "hey, this makes the lights blink" with the first
method than the second.

If I was stepping foward through an array for some nefarious purpose,
I would likely use the first method. Because the number of elements in
array is one greater than the index of the last element. Again for
readability.

However, consistancy counts. So if that's the bigest complaint, let it
be.

--
Don't let people drive you crazy when you know it's in walking
distance.



Sat, 08 Mar 2003 03:00:00 GMT  
 Style Question (slightly O.T.)


Quote:
>> So don't be too {*filter*} these guys!  However the < n test is a nearly
>> ubiquitous C idiom; ignoring such an idiom does seem to be some kind of
>> indication of a lack of C cultural awareness perhaps stemming from inexperience
>> or from programming in isolation.

>This is probably one of those "religious" style issues.

>I've always thought about the difference between "<" and "<=" as
>follows:

>< (from 0 to some n) is useful to understand how many times the
>loop will be run, as in (for i = 0;  i < 4; ++i) the loop will run
>4 times... This is IMHO a better, more universal way to present things.

><= (from 0 to some n-1) is useful to describe the bounds of the
>array on which the loop will operate. I like this technique much less,
>because you can't say how many times it runs in a glance (it takes
>your brain another few particles of a second to calculate n+1 ;)

>However, I've seen enough code using both techniques.

Eli - I agree that it's a religious issue.  I also agree that

for (i=0; i<n; i++)

is simpler by convention to discern that the loop executes n times.
In fact, if the beginning index value is 1 (as in fortran), I would be
inclined to write:

for (i=1; i<=n; i++)

This isn't the only problem with the code - only the cherry on top.
There's also a lot of cut and paste (multiple functions with a lot of
common code).

Also there is code like:

/* semi informative comment */
if(fA*fB<=0.)
{
    aaa             = a;
    bbb             = b;
    fAAA           = fA;
    zerofound = TRUE;

Quote:
}

(comment text changed to protect the guilty - code reproduced exactly
- nicely aligned & indented, but meaningless)

I guess the reason I asked was to try to get an insight as to WTF the
folks that wrote the code were thinking.  My guess is that they're
engineers that never got past FORTRAN IV but are forced to program in
C - I've seen that before from other vendors.  We paid a bundle for
this.

Kaz - your comments on summation are understood.   The more usual
summation would be:

      n
        ---
        \
        /    expr(i-1)
        ---
       i=1

At least that's how I was taught in math, EE, and physics.  It's only
the computer (Pascal, Basic, C, assembler?) weenies that want to start
counting with zero.
--
Jim Dalsimer



Sun, 09 Mar 2003 11:10:16 GMT  
 Style Question (slightly O.T.)

Quote:

> I'm reviewing some code provided by a vendor.  My first reaction is to
> vomit all over it.  Before I use my fire axe, I need your collective
> opinion.  What would you say if you consistently came across the
> following code in a for loop?

>         int i, n;

>         /* n is set to some valid value */
>         /* and then later we see */

>         for (i=0; i<=n-1; i++)

> my concern is the <= test on  i vs. n-1 instead of the normal (IMO)

>         for (i=0; i<n; i++)

What would I say?  I'd say you have too much time on your
hands.  Don't sweat the small stuff.

-- Bob Day



Sun, 09 Mar 2003 11:25:52 GMT  
 Style Question (slightly O.T.)


Quote:

[snip]
> Eli - I agree that it's a religious issue.  I also agree that

> for (i=0; i<n; i++)

> is simpler by convention to discern that the loop executes n times.

OK, this is based on really ancient stuff (C-64 6502 assembler).  I always
figured that i<n would get compiled to something like:

LDA i
CMP n
BNZ address

Essentially, the CMP instruction performed a subtract (i-n) without actually
storing the result--it just set flags indicating the characteristics of the
result, and one of them was the Z (negative) flag.

Now, i<=n-1 would have to do something like:

LDA n
DEC
STA temp ; or maybe TAX
LDA i
CMP temp ; or maybe CMP x, if such an instruction was available.
BEQ address
BNZ address

this would lead to much more bloated and slower code.  Now, let me say that
I never actually had a C compiler for my C-64, and often programmed it in
assembler directly, as that was the only way to get any speed out of it at
all.  But I always like to remind myself that the C is getting translated
into machine code somehow, and even if the compiler is intelligent enough to
optimize i<=n-1 into i<n, why bet on it?

So, I got curious just now and decided to generate a COD listing using MSVC
6.0 and the following function:

int test_function(int arg)
{
 int i;
 int n=arg;
 int result1=0;
 int result2=0;

 if (arg<2)
  {
  return 0;
  }

 for (i=0;i<=arg-1;i++)
  {
  result2+=i;
  }

 for (i=0;i<arg;i++)
  {
  result1+=i;
  }

 return result1+result2;

Quote:
}

The assembly listing is this:

PUBLIC _test_function
; COMDAT _test_function
_TEXT SEGMENT
_arg$ = 8
_test_function PROC NEAR    ; COMDAT

; 257  :  int i;
; 258  :  int n=arg;
; 259  :  int result1=0;
; 260  :  int result2=0;
; 261  :
; 262  :  if (arg<2)

 mov edx, DWORD PTR _arg$[esp-4]
 push esi
 push edi
 xor esi, esi
 xor edi, edi
 cmp edx, 2
 jge SHORT $L2048
 pop edi

; 263  :   {
; 264  :   return 0;

 xor eax, eax
 pop esi

; 278  : }

 ret 0
$L2048:

; 265  :   }
; 266  :
; 267  :  for (i=0;i<=arg-1;i++)

 lea ecx, DWORD PTR [edx-1]
 xor eax, eax
 test ecx, ecx
 jl SHORT $L2051
$L2049:

; 268  :   {
; 269  :   result2+=i;

 add edi, eax
 inc eax
 cmp eax, ecx
 jle SHORT $L2049
$L2051:

; 270  :   }
; 271  :
; 272  :  for (i=0;i<arg;i++)

 xor eax, eax
 test edx, edx
 jle SHORT $L2054
$L2052:

; 273  :   {
; 274  :   result1+=i;

 add esi, eax
 inc eax
 cmp eax, edx
 jl SHORT $L2052
$L2054:

; 275  :   }
; 276  :
; 277  :  return result1+result2;

 lea eax, DWORD PTR [edi+esi]
 pop edi
 pop esi

; 278  : }

 ret 0
_test_function ENDP
_TEXT ENDS

Now, x86 assembly is a LOT more complicated than 6502 was, which is why I
don't program it directly.  However, if you study this for a few minutes,
what you see is:

the <= loop performs one subtraction at the beginning and uses a jle.

The < loop performs no subtraction at the beginning and uses a jl.

Now, the subtraction adds an instruction which makes the code bigger, so
that's an obvious disadvantage.  The size of the loop code is the same.
More compelling is whether or not there is a performance difference between
jl and jle, because they will be executed many times during the loop.

Eliminating the extra setup code should be enough to convince anybody that
the < loop is better than the <= loop.  I'd be curious to hear what some
real x86 hackers have to say about the performance of jl vs. jle.

As it stands, it looks like the usual C way of doing things is better, at
least for this particular compiler.

--Steve

[snip]



Sun, 09 Mar 2003 03:00:00 GMT  
 Style Question (slightly O.T.)


Quote:
> Eli - I agree that it's a religious issue.  I also agree that

> for (i=0; i<n; i++)

> is simpler by convention to discern that the loop executes n times.
> In fact, if the beginning index value is 1 (as in FORTRAN), I would be
> inclined to write:

> for (i=1; i<=n; i++)

For the n-1 version, it is also a question if n is unsigned (size_t), and 0
is a valid value, e.g. describing the number of elements in a list or array.


Sun, 09 Mar 2003 03:00:00 GMT  
 Style Question (slightly O.T.)


[snip]

Quote:
>There's also a lot of cut and paste (multiple functions with a lot of
>common code).

[snip]

Is it possible this common code is machine-generated?

--
Mike Sherrill
Information Management Systems



Sun, 09 Mar 2003 03:00:00 GMT  
 Style Question (slightly O.T.)

Quote:



>[snip]
>>There's also a lot of cut and paste (multiple functions with a lot of
>>common code).
>[snip]

>Is it possible this common code is machine-generated?

>--
>Mike Sherrill
>Information Management Systems

Nope - no code generator used for this stuff.
--
Jim Dalsimer


Mon, 10 Mar 2003 03:00:00 GMT  
 
 [ 12 post ] 

 Relevant Pages 

1. slightly OT.

2. slightly OT: cross-platform binary compatibility?

3. Slightly OT: sorting

4. (slightly OT): Static Code Analysis Tool (C++)

5. C portability [slightly OT]

6. Slightly OT, but pertinant to ANSI C

7. slightly OT, but C# curious

8. (Slightly OT) General ActiveX information?

9. Slightly OT: VC++6, Dikumware 3.08 and Stingray

10. Slightly OT - Class Hierarchy

11. slightly OT.... Joining mpeg files

 

 
Powered by phpBB® Forum Software