Jump to content
  • Advertisement
Sign in to follow this  
Altourus

Code review, c++ opengl

This topic is 2949 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

It's been a while since I've used c++ for anything can someone take a look over my code for this program before I start anything too large and point out any rookie mistakes I made?

http://nickcrook.ca/ClothDemo/ClothDemoSource.zip

For reference, if it's running properly it should look like this



Also for future projects I'm going to use SDL but the template I started with was with glut and it had all the initialization code I needed.

Share this post


Link to post
Share on other sites
Advertisement
Particle.h
----------

* Use proper terminology in comments. It's 'Constructors' not 'Initializers'
* Write getters/setters for the member variables (don't leave them public). Seems like an annoying waste of time, but does offer the benefit of being able to stick a conditional breakpoint to find when a value is modified. (Makes debugging easier if you can always find out how 'force got that mental value'). They can be inline, but always move the implementations to underneath the class.
* Use doxygen commenting.
* No copy constructor / assignment operator.
* I prefer m_memberVariable to MemberVariable.

Particle.cpp
------------

I prefer:


//----------------------------------------------------------------------------------------------------------------------
Particle::Particle(Vector3f position, Vector3f acceleration)
: Position(position), Velocity(0,0,0), Acceleration(acceleration), Normal(0,0,0), Damping(0.4), InverseMass(1),
Stationary(false)
{
}



Fine otherwise....

Cloth.h
-------

* Inconsistant function names. use functionName(), not FunctionName().
* Get rid of the comments next to the header files, they don't add any value.
* Same problems as with Particle. Make public data Private, and add getters/setters.
* I'd also expect to see a const version of
	const Particle* getParticle(int x, int y) const;


* drawTriangle should be:
	static void drawTriangle(const Particle& p1, const Particle& p2, const Particle& p3, const Vector3f& color);
static Vector3f calculateTriangleCross(const Particle& p1, const Particle& p2, const Particle& p3);



Cloth.cpp
---------

*Inconstistant {}'s. Always use:


void func()
{
for()
{
}
}


* Since the particles are an array, you could use gl vertex arrays to render the data (using a stride of siezof(Particle). Would be a lot more efficient!
* Check the squared radius instead of the radius. (in ballCollision)
* OMP ?

void Cloth::update(float duration)
{
int num = (int)m_particles.size();
#pragma omp parallel for
for (int i=0; i<num; ++i)
{
particle->integrate(duration);
}
}


* Horrible:

for (int x = 0; x < Width; x++)
for (int y =0; y < Height; y++)
{
}



Better:

for (int x = 0; x != Width; ++x)
{
for (int y =0; y != Height; ++y)
{
}
}




Vector3
-------

* The methods could be inlined.
* Const const const!

float length() const;
Vector3f normalized() const;
void operator += (const Vector3f &vector);
void operator -= (const Vector3f &vector);
void operator *= (const float &scalar);
void operator /= (const float &scalar);
Vector3f operator/ (const float &scalar) const;
Vector3f operator- (const Vector3f &vector) const;
Vector3f operator+ (const Vector3f &vector) const;
Vector3f operator* (const float &scalar) const;
Vector3f operator- () const;
Vector3f cross(const Vector3f &vector) const;
float dot(const Vector3f &vector) const;

Share this post


Link to post
Share on other sites
Updated with consty goodness. Actually managed to get cpu usage down from 13% to 4% though its still taking 99% of the cpu on my old computer :P Thanks for the help.

Share this post


Link to post
Share on other sites
I'd vote for using const references in the constructors. This saves you one unnecessary copy, unless the compiler optimizes it.
Particle::Particle(const Vector3f &position, const Vector3f &acceleration) 
...

Quote:
Original post by RobTheBloke
for (int x = 0; x != Width; ++x)
{
for (int y =0; y != Height; ++y)
{
}
}


Does it actually matter whether you use '<' or '!=', beside preference?

Share this post


Link to post
Share on other sites
Quote:
Original post by Decrius
Does it actually matter whether you use '<' or '!=', beside preference?


Here, not really. The former is a weightier operation with some iterators so it's a performance thing there.

Share this post


Link to post
Share on other sites
Quote:
Original post by TheUnbeliever
Quote:
Original post by Decrius
Does it actually matter whether you use '<' or '!=', beside preference?


Here, not really. The former is a weightier operation with some iterators so it's a performance thing there.


Yea I was pretty sure != and <= would be equiv when compiled as their both just one assembly operation

Share this post


Link to post
Share on other sites
Quote:
Original post by Altourus
Quote:
Original post by TheUnbeliever
Quote:
Original post by Decrius
Does it actually matter whether you use '<' or '!=', beside preference?


Here, not really. The former is a weightier operation with some iterators so it's a performance thing there.


Yea I was pretty sure != and <= would be equiv when compiled as their both just one assembly operation


Those suggestions to inline, shouldn't be suggestions you have to inline those functions. Nearly all maths functions should be inlined Fast math library design

Also you could start your classes with your public interface instead of private, this is just a matter of taste but most C++ classes start with public interface because as a user of a header thats what you are interessted in

Share this post


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