patterncppMinor
Particle class for physics simulation
Viewed 0 times
simulationphysicsparticleforclass
Problem
This is a very lengthy class called
The code has very infrequent comments, so lots of it may not make sense. It also uses a 2D vector grid, because it is much faster than a normal vector. If the code appears to be unclear without the other files, then I will put the code on Github or something (but it uses SFML for drawing).
```
//Standard C++:
#include
#include
#include
#include
//My Headers:
#include "vector2.h"
#include "materialDatabase.h"
class Particle
{
private:
//Coords:
vector2 coords;
//Velocities:
vector2 velocity;
//Material:
std::string material;
//Expanding:
bool fillToolExpands = false;
//Mass:
double mass = 0;
//Bounciness:
double bounciness = 0.5;
public:
//All values:
void setAllValues(vector2, vector2, std::string, double, double);
void setEmpty();
//Copy Particle:
void copyParticle(Particle&);
//Coords:
void setCoords(vector2);
vector2 getCoords();
vector2 getPreciseCoords();
//Velocities:
void giveVelocity(vector2);
void setVelocity(vector2);
vector2 getVelocity();
//Material:
void setMaterial(std::string);
void setMaterialProperties(std::string);
std::string getMaterial();
//Expanding:
void setFillPartic
Particle, it relies on two other headers, one of them is a simple struct which contains an x and y (Vector2), and the other header gets a material's properties (materialProperties.h). The class is no where near done, and I am struggling to get my collision detection function to work. This means that currently the bounciness property has no real purpose, and odd things will happen in void checkParticleMovement(std::vector >& particleArray), for example, when there are two particles, one behind the other, moving at the same speed, the previous particle won't move on the grid.The code has very infrequent comments, so lots of it may not make sense. It also uses a 2D vector grid, because it is much faster than a normal vector. If the code appears to be unclear without the other files, then I will put the code on Github or something (but it uses SFML for drawing).
```
//Standard C++:
#include
#include
#include
#include
//My Headers:
#include "vector2.h"
#include "materialDatabase.h"
class Particle
{
private:
//Coords:
vector2 coords;
//Velocities:
vector2 velocity;
//Material:
std::string material;
//Expanding:
bool fillToolExpands = false;
//Mass:
double mass = 0;
//Bounciness:
double bounciness = 0.5;
public:
//All values:
void setAllValues(vector2, vector2, std::string, double, double);
void setEmpty();
//Copy Particle:
void copyParticle(Particle&);
//Coords:
void setCoords(vector2);
vector2 getCoords();
vector2 getPreciseCoords();
//Velocities:
void giveVelocity(vector2);
void setVelocity(vector2);
vector2 getVelocity();
//Material:
void setMaterial(std::string);
void setMaterialProperties(std::string);
std::string getMaterial();
//Expanding:
void setFillPartic
Solution
There is quite a bit of code here, so this is by no means a complete review. A few main points that caught my attention:
-
Your class could use some parameterized constructors. That would reduce the need for all the
-
-
I would advise keeping function parameter names in the function/method prototypes, as this adds to the documentation of the code.
-
You did not follow Const Correctness for the
-
Use `
-
Your class could use some parameterized constructors. That would reduce the need for all the
set* methods. The work done by setAllValues(), for instance, should clearly be done by a constructor.-
copyParticle() is also redundant with operator =, but some might prefer the more C-ish syntax of copying via a function.-
I would advise keeping function parameter names in the function/method prototypes, as this adds to the documentation of the code.
-
You did not follow Const Correctness for the
get*() methods. They are not altering any member state, so should be marked as const.-
Use `
for C++, is actually the C-language header file.
-
fillToolExpands is a strange name. The methods that manipulate it are setFillParticle() and isFillParticle(), so shouldn't it be called something like fillParticle or fillEnabled?
-
Another issue with fillToolExpands: Avoid declaring booleans in the middle of classes. A bool is usually 1 byte in size, so they will break data alignment and force the compiler to pad the bool to the size of a word, making your class larger in terms of memory. Placing bools always at the end of classes/structs will make the need for compiler-generated padding less frequent and won't require padding between fields.
-
Column alignment of similar lines is something that helps me digest code. This is certainly arguable, but I would change a block like this:
coords = startCoords;
velocity = startVelocity;
material = startMaterial;
mass = startMass;
bounciness = startBounciness;
To this:
coords = startCoords;
velocity = startVelocity;
material = startMaterial;
mass = startMass;
bounciness = startBounciness;
-
Don't use pow() to calculate the square of a number. That will call a function which can be an expensive one. Instead, just multiply the number by itself.
double r = pow(rV.x, 2) + pow(rV.y, 2);
double r2 = sqrt(r);
Simpler and faster:
double length = sqrt((rV.x * rV.x) + (rV.y * rV.y));
-
In the mathematical update of the particles, done by calculateGravitationalVelocity(), you use a few single letter variable names. Try to provide better and more descriptive names instead. E.g. r/r2 are actually the length of the vector. a is the acceleration of gravity. Use descriptive names and the comments can even be removed.
-
floor(), ceil(), fabs() and all functions declared by are all members of namespace std. Technically, compilers are not required to expose such functions in the global namespace, so for good portability, make sure to always prefix them with the std::` namespace resolution.Code Snippets
coords = startCoords;
velocity = startVelocity;
material = startMaterial;
mass = startMass;
bounciness = startBounciness;coords = startCoords;
velocity = startVelocity;
material = startMaterial;
mass = startMass;
bounciness = startBounciness;double r = pow(rV.x, 2) + pow(rV.y, 2);
double r2 = sqrt(r);double length = sqrt((rV.x * rV.x) + (rV.y * rV.y));Context
StackExchange Code Review Q#82424, answer score: 6
Revisions (0)
No revisions yet.