patterncppMinor
Mathematical Vector2 class implementation
Viewed 0 times
implementationvector2classmathematical
Problem
This is my first attempt ever to build a
Vector2.h
```
//VECTOR2 H
#ifndef VECTOR2_H
#define VECTOR2_H
//INCLUDES
#include
//DEFINE TYPES
typedef float float32;
//VECTOR2 CLASS
class Vector2
{
public:
//MEMBERS
float32 x;
float32 y;
//CONSTRUCTORS
Vector2(void);
Vector2(float32 xValue, float32 yValue);
Vector2(const Vector2 & v);
Vector2(const Vector2 * v);
//DECONSTRUCTOR
~Vector2(void);
//METHODS
void Set(float32 xValue, float32 yValue);
float32 Length() const;
float32 LengthSquared() const;
float32 Distance(const Vector2 & v) const;
float32 DistanceSquared(const Vector2 & v) const;
float32 Dot(const Vector2 & v) const;
float32 Cross(const Vector2 & v) const;
Vector2 & Normal();
Vector2 & Normalize();
//ASSINGMENT AND EQUALITY OPERATIONS
inline Vector2 & Vector2::operator = (const Vector2 & v) { x = v.x; y = v.y; return *this; }
inline Vector2 & Vector2::operator = (const float32 & f) { x = f; y = f; return *this; }
inline Vector2 & Vector2::operator - (void) { x = -x; y = -y; return *this; }
inline bool Vector2::operator == (const Vector2 & v) const { return (x == v.x) && (y == v.y); }
inline bool Vector2::operator != (const Vector2 & v) const { return (x != v.x) || (y != v.y); }
//VECTOR2 TO VECTOR2 OPERATIONS
inline const Vector2 Vector2::operator + (const Vector2 & v) const { return Vector2(x + v.x, y + v.y); }
inline const Vector2 Vector2::operator - (const Vector2 & v) const { return Vector2(x - v.x, y - v.y); }
inline const Vector2 Vector2::operator (const Vector2 & v) const { return Vector2(x v.x, y * v.y); }
inline const Vector2 Vector2::operator / (const Vector2 & v) const { return Vector2(x / v.x, y / v.y); }
//VECTOR2 TO THIS OPERATIONS
inline Vector2 & Vector2::operat
Vector2 class. I scoured the net for anything that might make this class more efficient but know I'm to the point where I'm ready to share. This way I can get advice on how I can improve it further. I wanted to do this before I make my Vector3 and Vector4 class.Vector2.h
```
//VECTOR2 H
#ifndef VECTOR2_H
#define VECTOR2_H
//INCLUDES
#include
//DEFINE TYPES
typedef float float32;
//VECTOR2 CLASS
class Vector2
{
public:
//MEMBERS
float32 x;
float32 y;
//CONSTRUCTORS
Vector2(void);
Vector2(float32 xValue, float32 yValue);
Vector2(const Vector2 & v);
Vector2(const Vector2 * v);
//DECONSTRUCTOR
~Vector2(void);
//METHODS
void Set(float32 xValue, float32 yValue);
float32 Length() const;
float32 LengthSquared() const;
float32 Distance(const Vector2 & v) const;
float32 DistanceSquared(const Vector2 & v) const;
float32 Dot(const Vector2 & v) const;
float32 Cross(const Vector2 & v) const;
Vector2 & Normal();
Vector2 & Normalize();
//ASSINGMENT AND EQUALITY OPERATIONS
inline Vector2 & Vector2::operator = (const Vector2 & v) { x = v.x; y = v.y; return *this; }
inline Vector2 & Vector2::operator = (const float32 & f) { x = f; y = f; return *this; }
inline Vector2 & Vector2::operator - (void) { x = -x; y = -y; return *this; }
inline bool Vector2::operator == (const Vector2 & v) const { return (x == v.x) && (y == v.y); }
inline bool Vector2::operator != (const Vector2 & v) const { return (x != v.x) || (y != v.y); }
//VECTOR2 TO VECTOR2 OPERATIONS
inline const Vector2 Vector2::operator + (const Vector2 & v) const { return Vector2(x + v.x, y + v.y); }
inline const Vector2 Vector2::operator - (const Vector2 & v) const { return Vector2(x - v.x, y - v.y); }
inline const Vector2 Vector2::operator (const Vector2 & v) const { return Vector2(x v.x, y * v.y); }
inline const Vector2 Vector2::operator / (const Vector2 & v) const { return Vector2(x / v.x, y / v.y); }
//VECTOR2 TO THIS OPERATIONS
inline Vector2 & Vector2::operat
Solution
Vector is a relatively common name (as is vector 2/3 etc). So you may need to make your include guards a bit more unique. I always put my stuff into my own namespace (which happens to match a domain I own to make things unique). I then include the namespace as part of the include guard. Alternatively you can generate a GUID that will also make sure it is unique.
Mine would look like:
Is it valid to create a Vector 2 with zero values? If so does that mean x/y default to 0.0 in which case the following 2
Could be redefined as (Though then it is possible to create a Vector2 with a single value (which may not be desirable).
Does your default copy constructor do anything special?
If not then I would let the compiler generated version by the one you use.
Not sure you want to be able to construct from a pointer. Very few classes allow this (apart from samart pointers). Does this mean you are taking ownership of the pointer or you will just make a copy of the vector.
I would drop this constructor.
Then the following code
Has to be modified like this (not a major change).
Don't put the void when you have an empty parameter list.
Also if the destructor does not do anything then use the compiler generated version.
Why do you have set method when the members are public.
All operators that have an assignment version are easier to define if you define them in terms of the assignment operator. It keeps the meaning consistent.
What I mean is: operator+ is easy to define in terms of operator+=
Since we are defining all the other operators you may want to define the comparison operator.
Now you can use the Vector2 as the key in a std::map.
//VECTOR2 H
#ifndef VECTOR2_H
#define VECTOR2_HMine would look like:
#ifndef BOBS_DOMAIN_VECTOR_2_H
#define BOBS_DOMAIN_VECTOR_2_H
namespace BobsDomain {Is it valid to create a Vector 2 with zero values? If so does that mean x/y default to 0.0 in which case the following 2
Vector2(void);
Vector2(float32 xValue, float32 yValue);Could be redefined as (Though then it is possible to create a Vector2 with a single value (which may not be desirable).
Vector2(float32 xValue = 0.0, float32 yValue = 0.0);Does your default copy constructor do anything special?
Vector2(const Vector2 & v);If not then I would let the compiler generated version by the one you use.
Not sure you want to be able to construct from a pointer. Very few classes allow this (apart from samart pointers). Does this mean you are taking ownership of the pointer or you will just make a copy of the vector.
Vector2(const Vector2 * v);I would drop this constructor.
Then the following code
Vector2* data = /* Get Vector2 */;
Vector2 copy(data);Has to be modified like this (not a major change).
Vector2* data = /* Get Vector2 */;
Vector2 copy(*data); // Notice the extra star.
// But this is not a big cost and simplifies the interface
// considerably as we do not need to wory about ownership
// semantics.Don't put the void when you have an empty parameter list.
Also if the destructor does not do anything then use the compiler generated version.
~Vector2(void);Why do you have set method when the members are public.
void Set(float32 xValue, float32 yValue);All operators that have an assignment version are easier to define if you define them in terms of the assignment operator. It keeps the meaning consistent.
What I mean is: operator+ is easy to define in terms of operator+=
Vector2 const operator+ (Vector2 const& rhs) const {Vector2 result(*this); return result += rhs;}
Vector2& operator+=(Vector2 const& rhs) {x += v.x; y += v.y; return *this;}Since we are defining all the other operators you may want to define the comparison operator.
bool operator<(Vector const& rhs) {return (x < rhs.x) || ((x == rhs.x) && (y < rhs.y));}Now you can use the Vector2 as the key in a std::map.
Code Snippets
//VECTOR2 H
#ifndef VECTOR2_H
#define VECTOR2_H#ifndef BOBS_DOMAIN_VECTOR_2_H
#define BOBS_DOMAIN_VECTOR_2_H
namespace BobsDomain {Vector2(void);
Vector2(float32 xValue, float32 yValue);Vector2(float32 xValue = 0.0, float32 yValue = 0.0);Vector2(const Vector2 & v);Context
StackExchange Code Review Q#5856, answer score: 6
Revisions (0)
No revisions yet.