patterncppMinor
Simple Vector2D and 3D class
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:
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.
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 vectorAlso, 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
-
If you template also over the number of dimensions, the code can be much compacted:
and you can still declare
-
You should add some container-type interface, so that your class an be used with
-
You should implement the arithmetic operations with assign versions
note that the loop will be optimised away by any half decent compiler.
-
Implement division by scalar via multiplication, it's much more efficient:
-
There is no need for code using rvalue references (
-
Binary operations should return a value rather than a reference, e.g.
-
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
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.