Re: libitl-0.6.4 :A possible BUG? C# porting almost complete!

[Tarik FDIL <tfdil at sagma dot ma>]

Salam,

I'm not the author of this program too, but I confirm that there is a bug in 
the expression :

#define DEG_TO_RAD(A) (A * (PI/180.0))

It should be 

#define DEG_TO_RAD(A) ((A) * (PI/180.0))

with parentheses around the A variable.

You've already tell the reason : the precompiler translates expression like 
DEG_TO_RAD(x+y)  to x + y*(PI/180) and not (x+y)*(PI/180)

So if the define expression is like you wrote, it shoud be corrected. No need 
to convert the macro to function. A simple pair of parentheses is sufficient.

Best regards

Tarik Fdil


 
Le Mercredi 29 Mars 2006 07:16, Meor Ridzuan Meor Yahaya a écrit :
> Dear All,
>
> I was working on porting libitl to C# to make it run on .net platform
> . Right now I'm almost done with prayertime . I'll be working on hijri
> date converter later.
>
> During my work, I've found a possible bug. In astro.h, it has the
> following definition:
>
> #define DEG_TO_RAD(A) (A * (PI/180.0))
>
>
> Then in astro.c, it uses it as follows (in getRefraction function):
>
> part2 = 1 / tan(DEG_TO_RAD(sunAlt + (7.31/(sunAlt + 4.4)))) + 0.0013515;
>
> Since in C# there is no such thing as #define, I created a function as
> follows:
>
> double DEG_TO_RAD(double A){ return A*(PI/180);}
>
> However, the above statement for part2 yields different values. I've
> tried so many things to trace what is the problem. First, I compiles
> the sample program using Microsoft C++ toolkit (free compiler from MS,
> version 2003).  I thought that it might be a compiler bug. Then I
> downloaded gcc, ( Dev-C++ distribution), I got the same results as MS
> compiler. Finally I came to conclusion the the problematic statement
> is the following:
>
> DEG_TO_RAD(sunAlt + (7.31/(sunAlt + 4.4)))
>
> Since libitl uses #define statement, I think it has a side effects.
> Then , I tried 2 different approaches:
> 1. rearrage the statement to the following:
> temp=sunAlt + (7.31/(sunAlt + 4.4));
> DEG_TO_RAD(temp);
>
> 2. Convert the DEG_TO_RAD to a proper function.
>
> In both instances, I got the same results as my C# program. So, my
> conclusion is, the bug arises because of the use of #define statement.
> We can evaluate the statement properly by doing the following:
> DEG_TO_RAD(sunAlt + (7.31/(sunAlt + 4.4)))=
> ( sunAlt + (7.31/(sunAlt + 4.4))* (PI/180.0)),  => this how #define
> works, and how compiler replaces the statement, which, obviously shows
> that it is not the same as the above approaches.
>
> The whole thing causes time for Maghrib and Shurooq to be earlier a
> few minutes than my version. Since I'm not the author of this program
> and don't know much on how the calculation works, I hope others can
> check this out. Hope the auther can verify this, whether this is a
> real bug of intended to be like that. if this is a true bug, then my
> suggestion is to convert the #define statement into a real function,
> so that we don't get any surprises anymore.
>
> Lastly, if anyone want to give my port to C# a try , please let me
> know. I'll send to the library. The porting was done almost with
> minimal changes on the program layout, just to make it work.  Later
> I'll probably make more changes to make it more Object Oriented and
> more tailored to C# environment. (the porting was done in 2 days, btw)
>
> Wassalam.