HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Mathematical Vector2 class implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationvector2classmathematical

Problem

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

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

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

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.