CODEWITHSUNDEEP
cfunctionfloating-pointmacros
Gregory Fenn
Gregory Fenn

Reputation: 488

Implementing a float equality check in a C function-like macro: where's the bug?

EDIT: I'm a fool, the macro parameters should have been parenthesized too. Thanks for your quick reply! Sorry for the common error post.

I have the following main.c file

#include <stdio.h>

#define EPSILON (0.00000005f)

#define IsEqual(x,y) (    ( (x-y)*(x-y)  <= EPSILON  ) ? 1 : 0   )
//int IsEqual(float x, float y){ return  ( (x-y)*(x-y)  <= EPSILON  ) ? 1 : 0  ; }


int main(void)
{
    printf("\n10 == 3+3+4: %d\n", IsEqual(10, 3+3+4));
    printf("\n10 == (1/3.0f)*27 + 1: %d\n", IsEqual(10, (1/3.0f)*27 + 1));
    printf("\n10 == -1: %d\n", IsEqual(10, -1.0f));
    printf("\n10 == -1: %d\n", IsEqual(10, -1));
    printf("\n10 == 10: %d\n", IsEqual(10, 10));

    return 0;
}

The output of this is:

10 == 3+3+4: 0
10 == (1/3.0f)*27 + 1: 0
10 == -1: 0
10 == -1: 0
10 == 10: 1

which is clearly not the intention. But if I comment-out the function-like macro IsEqual(x,y) and instead uncomment the function int IsEqual(float x, float y), then I get the expected output:

10 == 3+3+4: 1
10 == (1/3.0f)*27 + 1: 1
10 == -1: 0
10 == -1: 0
10 == 10: 1

What is wrong with my macro definition? The compiler should implicit cast int * float to float or int - float to float, so that shouldn't be the bug. Also we can see that EPSILON isn't too small to represent as a small positive float32 because the code works correctly when I use a function rather than a fuction-like macro.

Any tips? I'm hoping to learn something about macros and/or floats today!

best ~Greg

Upvotes: 0

Views: 777

Answers (3)

0___________
0___________

Reputation: 67999

IMO it should be implemented like this because if you multiply you can loose the precision and numbers not equal will become equal.

In your algorithm you need to check the sign of the parameters as well

#define EPSILON (0.00000005f)
#define isEqual(x,y)     (fabs((x) - (y)) <= EPSILON)

int main() 
{
    printf("\n10 == 3+3+4: %d\n", isEqual(10, 3+3+4));
    printf("\n10 == (1/3.0f)*27 + 1: %d\n", isEqual(10, (1/3.0f)*27 + 1));
    printf("\n10 == -1: %d\n", isEqual(10.0, -1.0f));
    printf("\n10 == -1: %d\n", isEqual(10.0, -1));
    printf("\n10 == 10: %d\n", isEqual(10, 10));
}

Upvotes: 0

srilakshmikanthanp
srilakshmikanthanp

Reputation: 2399

Change this

#define IsEqual(x,y) (    ( x*x - y*y  <= EPSILON  ) ? 1 : 0   )

To

#define IsEqual(x,y) (    ( (x)*(x) - (y)*(y)  <= EPSILON  ) ? 1 : 0   )

Because Macro are expands at preprocessing time

First will expand to:

printf("\n10 == 3+3+4: %d\n", ( ( 10*10 - 3+3+4*3+3+4 <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == (1/3.0f)*27 + 1: %d\n", ( ( 10*10 - (1/3.0f)*27 + 1*(1/3.0f)*27 + 1 <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == -1: %d\n", ( ( 10*10 - -1.0f*-1.0f <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == 10: %d\n", ( ( 10*10 - 10*10 <= (0.00000005f) ) ? 1 : 0 ));

You an check this by -E with gcc will produce preprocessed source code.

Second will expand to:

printf("\n10 == 3+3+4: %d\n", ( ( (10)*(10) - (3+3+4)*(3+3+4) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == (1/3.0f)*27 + 1: %d\n", ( ( (10)*(10) - ((1/3.0f)*27 + 1)*((1/3.0f)*27 + 1) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == -1: %d\n", ( ( (10)*(10) - (-1.0f)*(-1.0f) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == -1: %d\n", ( ( (10)*(10) - (-1)*(-1) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == 10: %d\n", ( ( (10)*(10) - (10)*(10) <= (0.00000005f) ) ? 1 : 0 ));

Thanks.

Upvotes: 0

klutt
klutt

Reputation: 31419

You need to put parenthesis around the arguments:

(x)*(x) - (y)*(y)

This is what your current code looks like after the preprocessor:

printf("\n10 == 3+3+4: %d\n", ( ( 10*10 - 3+3+4*3+3+4 <= (0.00000005f) ) ? 1 : 0 ));

Upvotes: 2

Related Questions