code critique
I wrote the following code to find the area of a triangle:
inline float triarea(float x0, float y0, float x1,
float y1, float x2, float y2)
{
#define U1 (x1 - x0)
#define U2 (y1 - y0)
#define V1 (x2 - x0)
#define V2 (y2 - y0)
#define CROSSPROD U1*V2 - U2*V1
#define AREA 0.5 * fabs(CROSSPROD);
return AREA;
}
Later on, I realized that all the defines will be around later on. Any ideas on how to fix this while still making the code readable and fast?
mike
http://www.coolgroups.com/
inline float triarea(float x0, float y0, float x1, float y1, float x2, float y2){ return 0.5 * fabs((x1 - x0) * (y2 - y0) - (y1 - y0) * (x2 - x0));}
?
All your #defines are redundant - there's no need for them.
If I undef and the macros are already defined, they will still be redefined and then undefined.
If I use variables it will be slower.
If I expand the macros, it will be less clear.
What I think I'd like is some sort of local macro or one that's limited to one file. Does such a beast exist?
mike
http://www.coolgroups.com/
If I use variables it will be slower.
If I expand the macros, it will be less clear.
What I think I'd like is some sort of local macro or one that's limited to one file. Does such a beast exist?
mike
http://www.coolgroups.com/
Quote:Original post by mike74
#define U1 (x1 - x0)
#define U2 (y1 - y0)
#define V1 (x2 - x0)
#define V2 (y2 - y0)
#define CROSSPROD U1*V2 - U2*V1
#define AREA 0.5 * fabs(CROSSPROD);
NO. NO. *smack with rolled-up newspaper*
Quote:If I use variables it will be slower.
Really? Have you tried it, on a release build with full optimization?
Quote:If I expand the macros, it will be less clear.
If a person reading your code cannot understand or figure out this:
Quote:inline float triarea(float x0, float y0, float x1, float y1, float x2, float y2)
{
return 0.5 * fabs((x1 - x0) * (y2 - y0) - (y1 - y0) * (x2 - x0));
}
then they shouldn't be reading code in the first place.
inline float triarea(float x0, float y0, float x1,float y1, float x2, float y2){ const float u1 = x1 - x0; const float u2 = y1 - y0; const float v1 = x2 - x0; const float v2 = y2 - y0; const float crossprod = u1*v2 - u2*v1; const float area = 0.5 * fabs( crossprod ); return area; }
That's how I would write it. The const's are somehow overkill, but sometimes it helps catching bugs.
[Edit]Ok, I wouldn't write it that way, but it is better than using #define's.[/edit]
You're still creating all these unnecessary variables, which would impact performance.
mike
http://www.coolgroups.com/
mike
http://www.coolgroups.com/
Quote:Original post by mike74
You're still creating all these unnecessary variables, which would impact performance.
Again: have you tried this?
Quote:Original post by mike74
You're still creating all these unnecessary variables, which would impact performance.
No, I am creating CONSTANTS. Which to the compiler, are equivalent to the expressions with which I initialized them. They don't take up memory space during runtime.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement