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

Simple Vector2D and 3D class

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

Problem

So I implemented a simple vec2 and vec3 classes, I wanted to know what remarks you guys can give me to improve it.
I try to work with c++11 and 14 so remarks on that also will be really great.

I tried to support move semantics but I don't really know if I did a really good job at it.

You can create a simple vector like so:

vmath::vec2<> v1{1.4f, 1.6f}; // This will create a float vector
vmath::vec2 v2{5, 4}; // This will create a int vector


Also, take a look at the getRandomVector{2/3} function, should it be encapsulated? Does the code itself is good?

EDIT: By the way, I will use it for a game I am making, so If you have any performance suggestion to make that would be great as well!

The classes (1 header file by the way):

```
#pragma once
#include
#include

namespace vmath {

#pragma region vec2
template
class vec2 {
public:
vec2(T newX, T newY) : x(newX), y(newY) {}

public:
inline T dot(vec2 const& rhs) {
return xrhs.x + yrhs.y;
}
public:
union {
struct { T x, y; };
struct { T r, g; };
struct { T s, t; };
};
};
#pragma endregion

#pragma region vec3
template
class vec3 {
public:
vec3(T newX, T newY, T newZ) : x(newX), y(newY), z(newZ) {}

public:
inline T dot(vec3 const& rhs) {
return xrhs.x + yrhs.y + z*rhs.z;
}

inline vec3 cross(vec3 const& rhs) {
return vec3(yrhs.z - zrhs.y,
zrhs.x - xrhs.z,
xrhs.y - yrhs.x);
}

public:
union {
struct { T x, y, z; };
struct { T r, g, b; };
struct { T s, t, p; };
};
};
#pragma endregion

#pragma region Operator overloading

#pragma region vec2 Overloading
// Operator +
template
vec2 operator+(vec2 lhs, vec2 const& rhs) {
lhs.

Solution

-
there seems to be no need/usage of the additional union members; a single data fields of the form T x[n] should be good enough.

-
If you template also over the number of dimensions, the code can be much compacted:

template
struct vec { T x[n]; /* ... */};


and you can still declare vec2 etc to be an alias:

template using vec2 = vec;


-
You should add some container-type interface, so that your class an be used with std type methods, for example for the dot product. I'm thinking in particular of members data(), size(), begin(), end(), value_type, etc. Even better: derive from std::array:

template
struct vec : std::array { /* ... */};


-
You should implement the arithmetic operations with assign versions

template
inline vec& vec::operator+=(vec const&arg) noexpect
{
  for(size_t n=0; n!=N; ++n)
    base::operator[](n)+=arg[n];
  return*this;
}


note that the loop will be optimised away by any half decent compiler.

-
Implement division by scalar via multiplication, it's much more efficient:

template
inline vec& vec::operator/=(T const&x) noexpect
{ return this->operator*=(1/x); }


-
There is no need for code using rvalue references (&& stuff), since you're not dealing with memory allocated on the heap.

-
Binary operations should return a value rather than a reference, e.g.

template
inline vec operator+(vec const&x, vec const&y) noexpect
{ /* ... */ }


-
The value-type template parameter doesn't need a default (as in your various operators).

-
You may want to implement the vector cross product (for N=3 only) by overloading the operator^ (but consider operator preference).

Code Snippets

template<typename T, size_t N>
struct vec { T x[n]; /* ... */};
template<typename T> using vec2 = vec<T,2>;
template<typename T, size_t N>
struct vec : std::array<T,N> { /* ... */};
template<typename T, size_t N>
inline vec<T,N>& vec<T,N>::operator+=(vec<T,N> const&arg) noexpect
{
  for(size_t n=0; n!=N; ++n)
    base::operator[](n)+=arg[n];
  return*this;
}
template<typename T, size_t N>
inline vec<T,N>& vec<T,N>::operator/=(T const&x) noexpect
{ return this->operator*=(1/x); }

Context

StackExchange Code Review Q#131240, answer score: 3

Revisions (0)

No revisions yet.