Comparison function in qsort()

[Spoon]

Hello,

I've come across the following comparison function used as the 4th
parameter in qsort()

int cmp(const void *px, const void *py)
{
const double *x = px, *y = py;
return (*x > *y) - (*x < *y);
}

It came "as-is" i.e. no comments.

Is it good style to write comparison functions that way?

Should the return values be made more explicit? Or would it be enough to
add comments explaining why the function (usually) works?

--
Regards, Spoon

[Michael Mair]

Spoon schrieb:

Hello,

I've come across the following comparison function used as the 4th
parameter in qsort()

int cmp(const void *px, const void *py)
{
const double *x = px, *y = py;
return (*x > *y) - (*x < *y);
}

It came "as-is" i.e. no comments.

Is it good style to write comparison functions that way?

Should the return values be made more explicit? Or would it be enough to
add comments explaining why the function (usually) works?

Depends. Possible points of view:
1) "Everyone" knows that relational operators in C return either 0
or 1, thus Everyone can figure it out.
2) If it is too complicated for Everyone, then it should be commented.
3) If it has to be explained so that Everyone understands it, then it
should not be written this way -- too clever for its own good.
4) Write the code as explicit and simple as possible:
- modern compilers on host PCs can optimise it better,
- there is no danger of misunderstandin g
5) As 4) but if you found out by measuring that cmp() is a bottleneck,
then try this out as one alternative; use the alternative which is
fastest and comment it clearly and explain why you did what you did.
6) As 5) but consider that probably the problem is using qsort(), as
well; you have indirection and function call overhead. It may be better
to write a dedicated double array sort. This makes the above construct
once more unnecessary. Use a well-tested out-of-the-book/from-the-CD
algorithm.

Which alternative you use depends on the people you work with
and the constraints you work under (coding guidelines etc.).
Cheers
Michael
--
E-Mail: Mine is an /at/ gmx /dot/ de address.

[Fred Kleinschmidt]

"Spoon" <none> wrote in message
news:44******** *************** @news.free.fr.. .

Hello,

I've come across the following comparison function used as the 4th
parameter in qsort()

int cmp(const void *px, const void *py)
{
const double *x = px, *y = py;
return (*x > *y) - (*x < *y);
}

It came "as-is" i.e. no comments.

Is it good style to write comparison functions that way?

Should the return values be made more explicit? Or would it be enough to
add comments explaining why the function (usually) works?

--
Regards, Spoon

It's (almost) never a good style to write ANY function with no comments at
all.
Most software development groups have some sort of coding standards.
Mine require ALL functions to specify what the function is for,
describe all parameters, and describe the return value.
In addition, such show-off coding without explaining why it works
can often lead to trouble later on when someone else with less
expertise tries to modify the code, or port it to a different language
(where, for example, boolean true is not 1).

In the above case, the coding is rather silly.
return (*x - *y);
works just as well, with or without the parentheses,
if the function is used only as a qsort() comparer.

If the answer MUST be -1, 0, or +1 for some reason (like it is used
by in some other comparison that requires that), it should be
documented as such.
--
Fred L. Kleinschmidt
Boeing Associate Technical Fellow
Technical Architect, Software Reuse Project

[Keith Thompson]

"Fred Kleinschmidt" <fr************ ******@boeing.c om> writes:

"Spoon" <none> wrote in message
news:44******** *************** @news.free.fr.. .

Hello,

I've come across the following comparison function used as the 4th
parameter in qsort()

int cmp(const void *px, const void *py)
{
const double *x = px, *y = py;
return (*x > *y) - (*x < *y);
}

It came "as-is" i.e. no comments.

Is it good style to write comparison functions that way?

Should the return values be made more explicit? Or would it be enough to
add comments explaining why the function (usually) works?

[snip] In the above case, the coding is rather silly.
return (*x - *y);
works just as well, with or without the parentheses,
if the function is used only as a qsort() comparer.

No, it wouldn't. Consider what happens if the subtraction overflows.
Or, since the operands are of type double, consider *x==0.5, *y==0.25;
the result of the subtraction is 0.25, which yields 0 when
(implicitly) converted to int.

--
Keith Thompson (The_Other_Keit h) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Flash Gordon]

Fred Kleinschmidt wrote:

"Spoon" <none> wrote in message
news:44******** *************** @news.free.fr.. .

Hello,

I've come across the following comparison function used as the 4th
parameter in qsort()

int cmp(const void *px, const void *py)
{
const double *x = px, *y = py; ^^^^^^

Note that this is comparing doubles
return (*x > *y) - (*x < *y);
}

It came "as-is" i.e. no comments.

Is it good style to write comparison functions that way?

Should the return values be made more explicit? Or would it be enough to
add comments explaining why the function (usually) works?

--
Regards, Spoon
It's (almost) never a good style to write ANY function with no comments at
all.
Most software development groups have some sort of coding standards.
Mine require ALL functions to specify what the function is for,
describe all parameters, and describe the return value.

Agreed.
In addition, such show-off coding without explaining why it works
can often lead to trouble later on when someone else with less
expertise tries to modify the code,
In this case, as the function is being passed to qsort its purpose is
fairly obvious, and anyone who could not at least work out what it is
doing and how it works is not ready to be working on their own as a
maintenance programmer.
or port it to a different language
(where, for example, boolean true is not 1).
When translating between two languages it is better in my experience to
look at the purpose of regions of code and write appropriate
replacements than to simply translate on a function by function basis.
In the above case, the coding is rather silly.
return (*x - *y);
works just as well, with or without the parentheses,
if the function is used only as a qsort() comparer.
No it doesn't. Or do you think that 0.1 - 0 is guaranteed to produce a
positive number rather than 0?

Your suggestion is not even guaranteed for int, since INT_MIN - 1 will
overflow, and the behaviour on overflow is undefined.
If the answer MUST be -1, 0, or +1 for some reason (like it is used
by in some other comparison that requires that), it should be
documented as such.

Agreed, but IMHO the code was probably written that way for correctness
rather than to produce exactly +0, -1 or 0.
--
Flash Gordon, living in interesting times.
Web site - http://home.flash-gordon.me.uk/
comp.lang.c posting guidelines and intro:
http://clc-wiki.net/wiki/Intro_to_clc

[Keith Thompson]

Keith Thompson <ks***@mib.or g> writes:

"Fred Kleinschmidt" <fr************ ******@boeing.c om> writes:

"Spoon" <none> wrote in message
news:44******** *************** @news.free.fr.. .

Hello,

I've come across the following comparison function used as the 4th
parameter in qsort()

int cmp(const void *px, const void *py)
{
const double *x = px, *y = py;
return (*x > *y) - (*x < *y);
}

It came "as-is" i.e. no comments.

Is it good style to write comparison functions that way?

Should the return values be made more explicit? Or would it be enough to
add comments explaining why the function (usually) works?

[snip]

In the above case, the coding is rather silly.
return (*x - *y);
works just as well, with or without the parentheses,
if the function is used only as a qsort() comparer.

No, it wouldn't. Consider what happens if the subtraction overflows.
Or, since the operands are of type double, consider *x==0.5, *y==0.25;
the result of the subtraction is 0.25, which yields 0 when
(implicitly) converted to int.

The code above, "return (*x > *y) - (*x < *y);", is also probably
better-behaved in the presence of infinities and NaNs, at least quiet
NaNs.

--
Keith Thompson (The_Other_Keit h) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[CBFalconer]

Fred Kleinschmidt wrote:

"Spoon" <none> wrote in message

I've come across the following comparison function used as the 4th
parameter in qsort()

int cmp(const void *px, const void *py)
{
const double *x = px, *y = py;
return (*x > *y) - (*x < *y);
}

It came "as-is" i.e. no comments.

Is it good style to write comparison functions that way?

Should the return values be made more explicit? Or would it be
enough to add comments explaining why the function (usually) works?

.... snip ...
In the above case, the coding is rather silly.
return (*x - *y);
works just as well, with or without the parentheses,
if the function is used only as a qsort() comparer.

No it doesn't. That is subject to overflow and undefined
behaviour. The recommended code may have been due to me, and
caters to avoidance of flushing of instruction streams. It
specifies the types being compared in exactly one place, and works
all the time.

IMO there are very few non-beginners who would not understand the
code. There are many coders who fail to understand the reasons for
the code.

--
"If you want to post a followup via groups.google.c om, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell. org/google/>
Also see <http://www.safalra.com/special/googlegroupsrep ly/>

[Robert Latest]

On Thu, 13 Apr 2006 00:37:39 +0100,
Flash Gordon <sp**@flash-gordon.me.uk> wrote
in Msg. <qi************ @news.flash-gordon.me.uk>

In this case, as the function is being passed to qsort its purpose is
fairly obvious, and anyone who could not at least work out what it is
doing and how it works is not ready to be working on their own as a
maintenance programmer.

A side note: I always like to name my qsort cmp functions starting with
'by_' because it makes for better English:

qsort(customer, ..., by_name);

robert

[Richard Bos]

Robert Latest <bo*******@yaho o.com> wrote:

Flash Gordon <sp**@flash-gordon.me.uk> wrote

In this case, as the function is being passed to qsort its purpose is
fairly obvious, and anyone who could not at least work out what it is
doing and how it works is not ready to be working on their own as a
maintenance programmer.

A side note: I always like to name my qsort cmp functions starting with
'by_' because it makes for better English:

qsort(customer, ..., by_name);

Hey, that's not a bad idea at all! I might adopt that.

Richard

[Richard Heathfield]

Richard Bos said:

Robert Latest <bo*******@yaho o.com> wrote:

A side note: I always like to name my qsort cmp functions starting with
'by_' because it makes for better English:

qsort(customer, ..., by_name);
Hey, that's not a bad idea at all!

Very good idea, in fact. Nice one, Robert.
I might adopt that.

Too late. I already have the papers right here, signed by a judge.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)