Simple Vector2D and 3D class

Question

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<int> 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 <random>
#include <limits>
namespace vmath {
#pragma region vec2
 template<typename T = float>
 class vec2 {
 public:
 vec2(T newX, T newY) : x(newX), y(newY) {}
 public:
 inline T dot(vec2 const& rhs) {
 return x*rhs.x + y*rhs.y;
 }
 public:
 union {
 struct { T x, y; };
 struct { T r, g; };
 struct { T s, t; };
 };
 };
#pragma endregion
#pragma region vec3
 template<typename T = float>
 class vec3 {
 public:
 vec3(T newX, T newY, T newZ) : x(newX), y(newY), z(newZ) {}
 public:
 inline T dot(vec3 const& rhs) {
 return x*rhs.x + y*rhs.y + z*rhs.z;
 }
 inline vec3<T> cross(vec3 const& rhs) {
 return vec3<T>(y*rhs.z - z*rhs.y,
 z*rhs.x - x*rhs.z,
 x*rhs.y - y*rhs.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<typename T = float>
 vec2<T> operator+(vec2<T> lhs, vec2<T> const& rhs) {
 lhs.x = lhs.x + rhs.x;
 lhs.y = lhs.y + rhs.y;
 return lhs;
 }
 template<typename T = float>
 vec2<T>& operator+(const vec2<T>& lhs, vec2<T>&& rhs) {
 rhs.x = lhs.x + rhs.x;
 rhs.y = lhs.y + rhs.y;
 return rhs;
 }
 // Operator -
 template<typename T = float>
 vec2<T> operator-(vec2<T> lhs, vec2<T> const& rhs) {
 lhs.x = lhs.x - rhs.x;
 lhs.y = lhs.y - rhs.y;
 return lhs;
 }
 template<typename T = float>
 vec2<T>& operator-(const vec2<T>& lhs, vec2<T>&& rhs) {
 rhs.x = lhs.x - rhs.x;
 rhs.y = lhs.y - rhs.y;
 return rhs;
 }
 // Operator *
 template<typename T = float>
 vec2<T> operator*(vec2<T> lhs, T const& rhs) {
 lhs.x = lhs.x * rhs;
 lhs.y = lhs.y * rhs;
 return lhs;
 }
 template<typename T = float>
 vec2<T> operator*(const vec2<T>& lhs, T&& rhs) {
 return vec2<T>(lhs.x * rhs, lhs.y * rhs);
 }
 // Operator /
 template<typename T = float>
 vec2<T> operator/(vec2<T> lhs, T const& rhs) {
 lhs.x = lhs.x / rhs;
 lhs.y = lhs.y / rhs;
 return lhs;
 }
 template<typename T = float>
 vec2<T> operator/(const vec2<T>& lhs, T&& rhs) {
 return vec2<T>(lhs.x / rhs, lhs.y / rhs);
 }
 template<typename T = float>
 bool operator==(const vec2<T>& lhs, const vec2<T>& rhs) {
 return lhs.x == rhs.x && lhs.y == rhs.y;
 }
 template<typename T = float>
 bool operator==(const vec2<T>& lhs, vec2<T>&& rhs) {
 return lhs.x == rhs.x && lhs.y == rhs.y;
 }
 template<typename T = float>
 bool operator!=(const vec2<T>& lhs, const vec2<T>& rhs) {
 return !(lhs == rhs);
 }
 template<typename T = float>
 bool operator!=(const vec2<T>& lhs, vec2<T>&& rhs) {
 return !(lhs == rhs);
 }
#pragma endregion
#pragma region vec3 Overloading
 // Operator +
 template<typename T = float>
 vec3<T> operator+(vec3<T> lhs, vec3<T> const& rhs) {
 lhs.x = lhs.x + rhs.x;
 lhs.y = lhs.y + rhs.y;
 lhs.z = lhs.z + rhs.z;
 return lhs;
 }
 template<typename T = float>
 vec3<T>& operator+(const vec3<T>& lhs, vec3<T>&& rhs) {
 rhs.x = lhs.x + rhs.x;
 rhs.y = lhs.y + rhs.y;
 rhs.z = lhs.z + rhs.z;
 return rhs;
 }
 // Operator -
 template<typename T = float>
 vec3<T> operator-(vec3<T> lhs, vec3<T> const& rhs) {
 lhs.x = lhs.x - rhs.x;
 lhs.y = lhs.y - rhs.y;
 lhs.z = lhs.z - rhs.z;
 return lhs;
 }
 template<typename T = float>
 vec3<T>& operator-(const vec3<T>& lhs, vec3<T>&& rhs) {
 rhs.x = lhs.x - rhs.x;
 rhs.y = lhs.y - rhs.y;
 rhs.z = lhs.z - rhs.z;
 return rhs;
 }
 // Operator *
 template<typename T = float>
 vec3<T> operator*(vec3<T> lhs, T const& rhs) {
 lhs.x = lhs.x * rhs;
 lhs.y = lhs.y * rhs;
 lhs.z = lhs.z * rhs;
 return lhs;
 }
 template<typename T = float>
 vec3<T> operator*(const vec3<T>& lhs, T&& rhs) {
 return vec3<T>(lhs.x * rhs, lhs.y * rhs, lhs.z * rhs);
 }
 // Operator /
 template<typename T = float>
 vec3<T> operator/(vec3<T> lhs, T const& rhs) {
 lhs.x = lhs.x / rhs;
 lhs.y = lhs.y / rhs;
 lhs.z = lhs.z / rhs;
 return lhs;
 }
 template<typename T = float>
 vec3<T> operator/(const vec3<T>& lhs, T&& rhs) {
 return vec3<T>(lhs.x / rhs, lhs.y / rhs, lhs.z / rhs);
 }
 template<typename T = float>
 bool operator==(const vec3<T>& lhs, const vec3<T>& rhs) {
 return lhs.x == rhs.x && lhs.y == rhs.y && lhs.z == rhs.z;
 }
 template<typename T = float>
 bool operator==(const vec3<T>& lhs, vec3<T>&& rhs) {
 return lhs.x == rhs.x && lhs.y == rhs.y && lhs.z == rhs.z;
 }
 template<typename T = float>
 bool operator!=(const vec3<T>& lhs, const vec3<T>& rhs) {
 return !(lhs == rhs);
 }
 template<typename T = float>
 bool operator!=(const vec3<T>& lhs, vec3<T>&& rhs) {
 return !(lhs == rhs);
 }
#pragma endregion
#pragma endregion
#pragma region Utils
 static std::random_device rd;
 static std::mt19937 gen(rd());
 template<typename T, typename std::enable_if<
 std::is_integral<T>::value>::type* = nullptr>
 vec2<T> getRandomVector2(T min = std::numeric_limits<T>::min(),
 T max = std::numeric_limits<T>::max()) {
 std::uniform_int_distribution<T> dist(min, max);
 return vec2<T>(dist(gen), dist(gen));
 }
 template<typename T, typename std::enable_if<
 std::is_floating_point<T>::value>::type* = nullptr>
 vec2<T> getRandomVector2(T min = std::numeric_limits<T>::min(),
 T max = std::numeric_limits<T>::max()) {
 std::uniform_real_distribution<T> dist(min, max);
 return vec2<T>(dist(gen), dist(gen));
 }
 template<typename T, typename std::enable_if<
 std::is_integral<T>::value>::type* = nullptr>
 vec3<T> getRandomVector3(T min = std::numeric_limits<T>::min(),
 T max = std::numeric_limits<T>::max()) {
 std::uniform_int_distribution<T> dist(min, max);
 return vec3<T>(dist(gen), dist(gen), dist(gen));
 }
 template<typename T, typename std::enable_if<
 std::is_floating_point<T>::value>::type* = nullptr>
 vec3<T> getRandomVector3(T min = std::numeric_limits<T>::min(),
 T max = std::numeric_limits<T>::max()) {
 std::uniform_real_distribution<T> dist(min, max);
 return vec3<T>(dist(gen), dist(gen), dist(gen));
 }
#pragma endregion
}

• Walter: it's unclear what you need the union members other than x,y for.
• Naor Hadar: @Walter Its because sometimes I can refer the x,y,z as rgb (the color values), it makes more sense to refer to "Red" as r instead of x, for example. (The other value (stp) is for texture values)

Answer 1

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

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

   template<typename T, size_t N>
   struct vec { T x[n]; /* ... */}; 
   

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

   template<typename T> using vec2 = vec<T,2>;
   

3. 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<typename T, size_t N>
   struct vec : std::array<T,N> { /* ... */}; 
   

4. You should implement the arithmetic operations with assign versions

   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;
   }
   

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

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

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

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

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

   template<typename T, size_t N>
   inline vec<T,N> operator+(vec<T,N> const&x, vec<T,N> const&y) noexpect
   { /* ... */ }
   

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

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

• Naor Hadar: 1. I wanted to "replicate" the GLSL style of vec2/3/4, forgot to mention that, sorry. 2. acceptable, but because I want to replicate GLSL, there is a way around it to use xyzw components with just one class? 3. I want to avoid deriving from std containers because of their allocations, but the functions sound good. 4. Should have added those, thanks! 5. Cool! Didn't think about that!
• Naor Hadar: 6. Great! should rvalue references be used only when dealing with memory allocated on the heap? 7. Will remove the reference. 8. Oops, thanks! 9. Will be considered!
• Naor Hadar: By the way, #6, If I do something like that v1 * vmath::vec2<int>{1,2} does it still apply? (I mean the use of rvalues)
• Walter: @NaorHadar 1. std::array<> has no allocations. 2. you can define vec2, vec3, etc to be aliases. 3. rvalue references are mostly useful for avoiding copying data allocated on the heap and then deleting the original. What is GLSL? Also what is this weird #pragma stuff about? (why not using proper documentation, either doxygen-style or otherwise?)
• Naor Hadar: This is GLSL, and I never used doxygen, I will read about it. Thanks