Jump to content
  • Advertisement
Sign in to follow this  
mike74

code critique

This topic is 4845 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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/

Share this post


Link to post
Share on other sites
Advertisement

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.

Share this post


Link to post
Share on other sites
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/

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

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]

Share this post


Link to post
Share on other sites
You're still creating all these unnecessary variables, which would impact performance.

mike
http://www.coolgroups.com/

Share this post


Link to post
Share on other sites
Quote:
Original post by mike74
You're still creating all these unnecessary variables, which would impact performance.

Again: have you tried this?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

This topic is 4845 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Guest
This topic is now closed to further replies.
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!