Mathematical Vector2 class implementation

Question

This is my first attempt ever to build a 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 <math.h>
//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::operator += (const Vector2 & v) { x += v.x; y += v.y; return *this; }
inline Vector2 & Vector2::operator -= (const Vector2 & v) { x -= v.x; y -= v.y; return *this; }
inline Vector2 & Vector2::operator *= (const Vector2 & v) { x *= v.x; y *= v.y; return *this; }
inline Vector2 & Vector2::operator /= (const Vector2 & v) { x /= v.x; y /= v.y; return *this; }
//SCALER TO VECTOR2 OPERATIONS
inline const Vector2 Vector2::operator + (float32 v) const { return Vector2(x + v, y + v); }
inline const Vector2 Vector2::operator - (float32 v) const { return Vector2(x - v, y - v); }
inline const Vector2 Vector2::operator * (float32 v) const { return Vector2(x * v, y * v); }
inline const Vector2 Vector2::operator / (float32 v) const { return Vector2(x / v, y / v); }
//SCALER TO THIS OPERATIONS
inline Vector2 & Vector2::operator += (float32 v) { x += v; y += v; return *this; }
inline Vector2 & Vector2::operator -= (float32 v) { x -= v; y -= v; return *this; }
inline Vector2 & Vector2::operator *= (float32 v) { x *= v; y *= v; return *this; }
inline Vector2 & Vector2::operator /= (float32 v) { x /= v; y /= v; return *this; }
};
#endif
//ENDFILE

Vector2.cpp

//VECTOR2 CPP
#include "Vector2.h"
//CONSTRUCTORS
Vector2::Vector2(void) : x(0), y(0) { }
Vector2::Vector2(float32 xValue, float32 yValue) : x(xValue), y(yValue) { }
Vector2::Vector2(const Vector2 & v) : x(v.x), y(v.y) { }
Vector2::Vector2(const Vector2 * v) : x(v->x), y(v->y) { }
//DECONSTRUCTOR
Vector2::~Vector2(void) { }
//METHODS
void Vector2::Set(float32 xValue, float32 yValue) { x = xValue; y = yValue; }
float32 Vector2::Length() const { return sqrt(x * x + y * y); }
float32 Vector2::LengthSquared() const { return x * x + y * y; }
float32 Vector2::Distance(const Vector2 & v) const { return sqrt(((x - v.x) * (x - v.x)) + ((y - v.y) * (y - v.y))); }
float32 Vector2::DistanceSquared(const Vector2 & v) const { return ((x - v.x) * (x - v.x)) + ((y - v.y) * (y - v.y)); }
float32 Vector2::Dot(const Vector2 & v) const { return x * v.x + y * v.y; }
float32 Vector2::Cross(const Vector2 & v) const { return x * v.y + y * v.x; }
Vector2 & Vector2::Normal() { Set(-y, x); return *this; }
Vector2 & Vector2::Normalize()
{
if(Length() != 0)
{
 float32 length = LengthSquared();
 x /= length; y /= length;
 return *this;
}
x = y = 0;
return *this;
}
//ENDFILE

First Edit:

Changed Cross to OrthoVector:

Vector2 & Vector2::Ortho() { Set(-y, x); return *this; }

Changed Normal code to actually get the normal of the vector:

Vector2 & Vector2::Normal() { float32 len = Length(); Set(x / len, y / len); return *this; }

Second Edit:

Changed the normal code again to check for division by zero:

Vector2 & Vector2::Normal()
{
if(Length() != 0)
{
 float32 len = Length();
 x /= len; y /= len;
 return *this;
}
x = y = 0;
return *this;
}

Got rid of my Pointer Constructor and my Empty Deconstructor as well.

• Mranz: Cross product is meaningless in the context that you have created it in for 2d vectors. If you are going for the orthogonal vector, it is simply "(x, y) -> (y, -x)"
• Pubby: Define "better". Do you want fast? Usable? Safe?
• Liquid Wotter: faster and more usable.
• 200_success: Note that revising the code in the question, even as an addendum, is no longer accepted practice on Code Review. However, since this is an old question, we'll let it stand.

Answer 1

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.

//VECTOR2 H
#ifndef VECTOR2_H
#define VECTOR2_H

Mine 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.

• Liquid Wotter: I will defiantly be taking you up on the unique namespace tip. I kept the void and set constructors for the very reason you listed above. I will keep my &copy constructor the same but will get rid of the pointer constructor because it is your right its virtually the same as the &copy version. I will get rid of the deconstructor. I made the set method to make my easier. I forgot about those operators and will be adding them in. Wow thanks for all the advice.
• Liquid Wotter: Also I created my operator+ that way so you cant do this: (a + b) = c
• Loki Astari: @Liquid Wotter: Fixed my version of +=. I was typing quickly this morning.
• user3804: @LokiAstari, where is Unit-vector implementation? How to do that?
• Loki Astari: @BROY: Do you mean std::vector?

Answer 2

Cross product is meaningless in the context that you have created it in for 2d vectors. If you are going for the orthogonal vector, it is simply "(x, y) -> (y, -x)".

Your "Normal" function is actually returning the orthogonal vector. "Normal" is vectorland means "Vector of unit length in the same direction".

To calculate the real normal, it is

 ai + bj double mag = sqrt((a*a) + (b*b));
----------- or Vector2 normal(a / mag, b / mag);
| ai + bj |

• Liquid Wotter: That's what I thought, I read otherwise something else somewhere that confused me. I will be changing the Normal code and get rid of CrossProduct. I will also make a snippet for the OrthogonalVector.

Answer 3

One fundamental property of vectors you haven't represented: a scalar times a vector is a vector.

I wouldn't actually implement your vector * vector and vector / vector operations at all ... those are not commonly useful.

Actually there is a meaningful cross product in two dimensions, but the return value is a scalar, not a vector.

float32 Cross(const Vector2& a, const Vector2& b) {
 return a.x * b.y - a.y * b.x;
}

This brings up another point: I prefer free functions to member functions when the meaning is roughly symmetric in two variables, like Dot, Cross, or even operator+. Besides aesthetics, there's the fact that operator+(const X& a, const Y& b) can use implicit conversions on both operands but X::operator+(const Y& b) const can only use implicit conversions on b.

Answer 4

Provide alternatives to methods returning copies.

On such classes, I usually defines a setAdd and setSub method that fill the current vector with the result of an addition (resp subtraction).

void setAdd(const Vector2& v1, const Vector2& v2)
{
 x = v1.x + v2.x;
 y = v1.y + v2.y;
}

using this kind of method in place of operator+ spare a Vector2 copy. In certain cases this as a visible impact on code performance (see Havok Physics math lib).

Handle more use cases in normalize

When normalizing a vector you often do further computation using its previous norm, and you often need to set it to a desired norm. It's easy to modify the Normalize method for those use cases.

float32 Vector2::Normalize(float32 desiredNorm = 1.f)
{
 float32 len = Length();
 if(len < EPSILON)
 {
 x = y = 0.f;
 return 0.f;
 }
 else
 {
 float32 mult = desiredNorm/len;
 x *= mult; y *= mult;
 return len;
 }
 }

NB. I have also make the following changes:

• Only one computation of Length(), square root computation are expensive.
• Use a less-than-epsilon instead of a different-to-0 which can lead to various problem because of floating point precision, the value of EPSILON should be small (0.00001 for example).

Provide a truncate method

I've found this very usefull to define a truncate method that enforce an upper-bound on the vector's norm.

void Truncate(float32 upperBound)
{
 float32 sqrLen = SqrLength();
 if(sqrLen > upperBound * upperBound)
 {
 float32 mult = upperBound/sqrt(sqrLen);
 x *= mult; y *= mult;
 }
}

NB.

• You should probably assert that upperBound is positive.
• Again I'm trying to minimize the number of square root computed.