A newbie's code

[Ian Collins]

Keith Thompson posted:





Efficiency and clarity. And efficiency again.

Can you provide an example?

 

[Bill Pursell]

Keith Thompson posted:



If my own opinion is worth anything, I've never used NULL once in my code.

How do you terminate your lists?

 

[ena8t8si]

Flash Gordon posted:



I would frown upon the use of array subscripting rather than pointers.

Using arrays rather than pointers is often clearer
and also often produces faster code. If you measure
you may find yourself giving up some of your ideas
about which choice is better.

 

[ena8t8si]

Keith Thompson posted:



If my own opinion is worth anything, I've never used NULL once in my code.

So what you're saying is you've used NULL a lot?

 

[Herbert Rosenau]

Just a *few* suggestions:


You're executing TWO assignment statements unecessarily in the case
where one or both params are NULL. And you neednt set "j" at all
here, as it's always set in the for initializer part (whatever you call
the first for() thingamaggummy).


You are to be complimented n checking your parameters, wish MicroSoft
did so with any regularity in their examples.

Microsoft does so many things absolutely wrong that having M$ as
excample makes no sense except as bad one.

But would it kill you to write clearer code, ala:

if( a == NULL || b == NULL ) ....

Don't bother on style. It makes no sense here as one cand find more
sources using the short form as the long form you uses.

Yes, I know !a is a well established C idiom. Even highly regarded
snippets of code are full of that kind of thing. Pls ponder the
relative clarity of each, especially when you get a call at 2:21AM to
fix your program.

Boa, ey! Not anybody is a beginner like you who can't read and
understund most common used style. As sayed above: don't bother on
style.

"return" is a mighty hand thing, but it's in a sense a amorphous goto.
Consider having just one exit point, at the bottom of the function. It
makes adding debug lines, assertions, and return value settings sooo
much clearer.

Anyone who had learned C in practise will follow the styles bignners
gets telled.

In practise there are 2 different forms of functions:

1. return on error quickly to forget about that for the rest of the
whole function but declaring the callee: something is faulty.
Multiple returns flagging error always. Not returned yet means that
anything done already is o.k. so far.
The last return is complete success of anything the function has to
do.

2. return on 'work done' already.
Single return at end is reached only when an error shows that the
whole work failed.
It makes only formalists happy tho have 50 cascaded ifs or checking
flags again and again and again endless only to avoid a single
retuern statement. It ends up in code hard to read and hard to
understund where a simple return cleans up well.

Which form is used depends on the question: what is easier to reach?
- complete success? return when success, left error open.
- faiture? return the failture found, left work open
as it can't be done on the error reached.

Hungin inside the function only because one has to reach the one only
return statement makes it hard to understund the whole function as
such.

for(;a;i++)

This is fine, a really puristt would suggest initializing "i" here,
just in case someone ever adds code between the initialization and this
point and one loses track of wat's in "i".

Don't bother on style. Yes, it makes sense to initialise the control
variables of the loop in the initialiser list. But again it's mostenly
only a style question.

Also a bubbly-headed coder might suggest "a != EndOfString" or even
"a IsNot EndOfString". Much clearer when the F133 payroll program
for the whole Skunk Works Division of Lockheed isnt running (a friend
of mine was called at 4 AM to fix that).
Yes, it takes a smidgen more typing, but disk space is not exactly
expensive these days.

Again: don't bother on style.

Otherwise okay.

To OP: Don't be so miserly on spaces!

e.g.: for ( ; a; i++)

is more readable as yours.

--
Tschau/Bye
Herbert

Visit http://www.ecomstation.de the home of german eComStation
eComStation 1.2 Deutsch ist da!

 

[Richard Bos]

Just a *few* suggestions:

[ Can you please not snip attributions? Thanks. ]

You're executing TWO assignment statements unecessarily in the case
where one or both params are NULL.

No, he's not. He's initialising.

And you neednt set "j" at all here,

And, surprise, he isn't.

But would it kill you to write clearer code, ala:

if( a == NULL || b == NULL ) ....

Yes, I know !a is a well established C idiom. Even highly regarded
snippets of code are full of that kind of thing. Pls ponder the
relative clarity of each, especially when you get a call at 2:21AM to
fix your program.

How amusing that you use "Pls" while telling the OP not to use !a.

If you can't understand perfectly normal C at two in the morning, don't
answer the phone at two in the morning. !a will be the least of your
worries.

"return" is a mighty hand thing, but it's in a sense a amorphous goto.
Consider having just one exit point, at the bottom of the function.

IOW, consider complicating the layout of your function for the paltry
purpose of pleasing Niklaus Wirth. Bah.

for(;a;i++)

Also a bubbly-headed coder might suggest "a != EndOfString" or even
"a IsNot EndOfString".

That would be a truly phenomenally bubbly-headed coder. If you want to
hack on the Bourne shell, you know where to find it.

Richard

 

[Richard Bos]

main() posted:


You should decide whether you want the programmer to be able to supply this
function with null pointers. If you forbid this, then use assertions:

assert(a);
assert(b);

While developing only! Do not, ever, let a user see the result of an
assertion failure. They get remarkably stroppy when all their work is
dumped in the bit bucket with nothing to show for it but a cryptic error
message, and that for no better reason than that the programmer was too
lazy to use proper error checking instead of assert(). NDEBUG is your
friend, here.

Richard

 

[Frederick Gotham]

Ian Collins posted:

Can you provide an example?

Firstly, let's take two snippets which achieve the same objective:

Snippet (1):

size_t i;

for(i = 0; i != len; ++i)
{
arr += 77;
}

Snippet (2):

int *p = arr;
int const *const pover = arr + len;

do *p++ += 77;
while(p != pover);

The body of the loop in Snippet (1) is equivalent to:

*(arr + i) += 77;

Upon each iteration, "i" is added to the address of the first element, and
then this address is dereferenced and assigned to.

Looking at the body of the loop in Snippet (2), "p" is dereferenced and
assigned to, and then "p" is incremented.

Snippet (2) is more efficent, and is cleaner in my opinion -- I much prefer
incrementing pointers than playing around with array indexes (but maybe
this is a matter of personal taste).

Of course, an optimising compiler *might* produce the same code for both,
but I prefer Snippet (2), as I like to write efficient code, and I also
think it's cleaner.

 

[Frederick Gotham]

posted:

Using arrays rather than pointers is often clearer
and also often produces faster code. If you measure
you may find yourself giving up some of your ideas
about which choice is better.

I don't see how array subscripting could be faster. Elsethread, I have
explained how the pointer version is likely to be faster.

Can you give an example of where array subscripting would be faster for
iterating through an array?

 

[Frederick Gotham]

Bill Pursell posted:

How do you terminate your lists?

I don't understand the question, please be more specific.

 

[Frederick Gotham]

Herbert Rosenau posted:

Microsoft does so many things absolutely wrong that having M$ as
excample makes no sense except as bad one.

I particularly loathe their use of "memset" for initialising arrays:

float array1[56];
int *array2[56];

memset(array1,0,sizeof array1);
memset(array2,0,sizeof array2);

Not only is it stupid, it's non-portable too.

Boa, ey! Not anybody is a beginner like you who can't read and
understund most common used style. As sayed above: don't bother on
style.

I tend to get jumped on whenever I bring up that argument.

 

[Frederick Gotham]

Ancient_Hacker posted:

Pls ponder the relative clarity of each, especially when you get a call
at 2:21AM to fix your program.

I don't answer the phone after about 10pm -- I find life to be more pleasant
that way.

 

[Frederick Gotham]

Richard Bos posted:

While developing only! Do not, ever, let a user see the result of an
assertion failure. They get remarkably stroppy when all their work is
dumped in the bit bucket with nothing to show for it but a cryptic error
message, and that for no better reason than that the programmer was too
lazy to use proper error checking instead of assert(). NDEBUG is your
friend, here.

I was implying that it would be a compile-time programmer error if either
pointer were null, not a runtime error.

When you provide somebody with a completed program which you've written, all
programming errors should have been remedied, and the user should notice no
difference whether NDEBUG is defined or not (except of course for a minor
speed difference).

 

[Bill Pursell]

Bill Pursell posted:



I don't understand the question, please be more specific.

It's pretty standard to implement a list like this:

struct foo {
void *data;
struct foo * next;
};

And then traverse the list via:

for ( a_foo = head; a_foo /* != NULL*/; a_foo = a_foo->next)
;

That will only work if the first element that is inserted into
the list needs has had the assignment:
a_foo->next = NULL;
(eg, the tail of the list must point to NULL).
There are other ways to implement this, of course, but
the most natural is to use NULL as the terminator.

 

[Flash Gordon]

Ian Collins posted:

Can you provide an example?

Firstly, let's take two snippets which achieve the same objective:

Snippet (1):

size_t i;

for(i = 0; i != len; ++i)
{
arr += 77;
}

Snippet (2):

int *p = arr;
int const *const pover = arr + len;

do *p++ += 77;
while(p != pover);

The body of the loop in Snippet (1) is equivalent to:

*(arr + i) += 77;

Upon each iteration, "i" is added to the address of the first element, and
then this address is dereferenced and assigned to.

You are assuming that the optimiser does not do anything. Optimisers
have certainly been dealing with things like this for over 10 years
(IIRC I saw this kind of strength reduction mentioned in a manual for a
C compiler in 1995).

Looking at the body of the loop in Snippet (2), "p" is dereferenced and
assigned to, and then "p" is incremented.

Snippet (2) is more efficent,

You are assuming that the optimiser does not convert both to the same
machine code. You are also assuming that the processor does not have
index operations which would make the array form require only one
instruction or that said machine code instructions are slower.

> and is cleaner in my opinion -- I much prefer
incrementing pointers than playing around with array indexes (but maybe
this is a matter of personal taste).

That, indeed, is true. It is a matter of personal taste.

Of course, an optimising compiler *might* produce the same code for both,
but I prefer Snippet (2), as I like to write efficient code, and I also
think it's cleaner.

It isn't necessarily more efficient even if converted directly in to
assembler with no optimisation. Personally I would bin any compiler so
bad it could not reduce array operations to pointer operations if the
pointer version was more efficient. As other have pointed out it can be
*harder* for the compiler to optimise pointer operations because it has
to be able to prove whether aliasing is occurring or not.

The best rule is to always write what you consider to be clearest, not
what you think might save a fraction off the execution time. Since you
consider the pointer version to be clearer *that* is a valid reason for
preferring the pointer versions, but when someone considers the array
index version to be clearer there is nothing intrinsically wrong with it.

 

[ena8t8si]

posted:



I don't see how array subscripting could be faster. Elsethread, I have
explained how the pointer version is likely to be faster.

Can you give an example of where array subscripting would be faster for
iterating through an array?

size_t my_strlen_array( const char *s ){
unsigned r = 0;
while (s[r]) r++;
return r;
}

size_t my_strlen_pointer( const char *s ){
unsigned r = 0;
while (*s++) r++;
return r;
}

size_t my_strlen_pointer_2( const char *const s_0 ){
const char *s = s_0;
while (*s) s++;
return s-s_0;
}

My measurements show my_strlen_array is faster than
either of the pointer versions. YMMV, of course.

 

[Frederick Gotham]

Flash Gordon posted:

You are assuming that the optimiser does not convert both to the same
machine code. You are also assuming that the processor does not have
index operations which would make the array form require only one
instruction or that said machine code instructions are slower.

Optimisers don't give the programmer freedom to write code however he/she
pleases. Some code is always going to be more efficient than other code.

It isn't necessarily more efficient even if converted directly in to
assembler with no optimisation. Personally I would bin any compiler so
bad it could not reduce array operations to pointer operations if the
pointer version was more efficient.

You critisize the compiler... Why not critisize the programmer?! The
programmer clearly could have used pointers if they desired.

As other have pointed out it can be *harder* for the compiler to
optimise pointer operations because it has to be able to prove whether
aliasing is occurring or not.

What is aliasing? I've never heard of it. (Not being sarcastic)

The best rule is to always write what you consider to be clearest, not
what you think might save a fraction off the execution time. Since you
consider the pointer version to be clearer *that* is a valid reason for
preferring the pointer versions, but when someone considers the array
index version to be clearer there is nothing intrinsically wrong with
it.

Indeed, both snippets achieve the same objective -- one is inherently more
efficient though.

 

[Frederick Gotham]

size_t my_strlen_array( const char *s ){
unsigned r = 0;
while (s[r]) r++;
return r;
}

size_t my_strlen_pointer( const char *s ){
unsigned r = 0;
while (*s++) r++;
return r;
}

size_t my_strlen_pointer_2( const char *const s_0 ){
const char *s = s_0;
while (*s) s++;
return s-s_0;
}

My measurements show my_strlen_array is faster than
either of the pointer versions. YMMV, of course.

How could that possibly be? I would have thought the fastest was:

#include <cassert>

size_t StrLength(char const *const start)
{
int dummy = (assert(start), 0);

char const *p = start;

while(*p++);

return p - start - 1;
}

 

[Frederick Gotham]

Bill Pursell posted:

There are other ways to implement this, of course, but
the most natural is to use NULL as the terminator.

I test a pointer like as follows:

if(p)
{
/* Stuff */
}

 

[Ian Collins]

Ian Collins posted:

Can you provide an example?

Firstly, let's take two snippets which achieve the same objective:

Snippet (1):

size_t i;

for(i = 0; i != len; ++i)
{
arr += 77;
}

Snippet (2):

int *p = arr;
int const *const pover = arr + len;

do *p++ += 77;
while(p != pover);

The body of the loop in Snippet (1) is equivalent to:

*(arr + i) += 77;

Upon each iteration, "i" is added to the address of the first element, and
then this address is dereferenced and assigned to.

Looking at the body of the loop in Snippet (2), "p" is dereferenced and
assigned to, and then "p" is incremented.

Snippet (2) is more efficent, and is cleaner in my opinion -- I much prefer
incrementing pointers than playing around with array indexes (but maybe
this is a matter of personal taste).

You are assuming a very naive compiler. For example, where your first
snippet is f1 and the second f2:

void main(void) {
int arr[10];

f1( arr, 10 );
f2( arr, 10 );
}

Is see the call to f1 optimised to:

leal 8(%esp),%edx
movl 8(%esp),%eax
addl $77,%eax
movl %eax,8(%esp)
addl $77,12(%esp)
addl $77,16(%esp)
addl $77,20(%esp)
addl $77,24(%esp)
addl $77,28(%esp)
addl $77,32(%esp)
addl $77,36(%esp)
addl $77,40(%esp)
addl $77,44(%esp)

Which is about as efficient as one can get.

Another point to note in these days of multiple core CPUs, it is much
easier for a compiler that supports OpenMP to add parallelism to snippet 1.