4
\$\begingroup\$

Just made a simple Vec class, nothing fancy but I am open to suggestions for improvements. Of course, this is not suppose to replace or used for the same tasks as std::vector. This is more like opencv's Vec and eventually I want to include this in a bigger project of mine.

#include <cmath>
#include <vector>
#include <initializer_list>
#include <cassert>
#define VEC_ASSERT(x) assert(x)
template<typename T, unsigned int C>
class Vec {
public:
 typedef T dataType;
 typedef T& dataType_ref;
 typedef const T& dataType_cref;
 // Empty Constructor
 Vec();
 // Single-Arg Constructor
 explicit Vec(T v);
 // From std::vector Constructor
 Vec(const std::vector<T>& v);
 // From std::initializer_list Constructor
 Vec(const std::initializer_list<T>& l);
 // Main Constructor
 template<typename ... Args>
 explicit Vec(T v, Args&& ... args);
 // Get vector dimensions
 unsigned int dim() const;
 // Get vector length
 double length() const;
 // Get vectors dist
 double dist(const Vec<T, C>& v) const;
 // Get the cross product (3D Vectors only)
 Vec<T, C> cross(const Vec<T, C>& v) const;
 // Get the dot product
 double dot(const Vec<T, C>& v) const;
 // Get ortho vector (2D vectors only)
 Vec<T, C> ortho() const;
 // Normalize vector values
 Vec<T, C> norm() const;
 // Rotate (2D Vectors only)
 Vec<T, C> rotate(double angle) const;
 // Rotate on x-axis (3D Vectors only)
 Vec<T, C> rotateX(double angle) const;
 // Rotate on y-axis (3D Vectors only)
 Vec<T, C> rotateY(double angle) const;
 // Rotate on z-axis (3D Vectors only)
 Vec<T, C> rotateZ(double angle) const;
 // Convert to std::vector
 std::vector<dataType> to_std_vector() const;
 // Cast
 template<typename TT, unsigned int CC = C>
 Vec<TT, CC> to() const;
 // Access vector values
 dataType_ref operator[](int index);
 dataType_ref operator()(int index);
 dataType_cref operator[](int index) const;
 dataType_cref operator()(int index) const;
 // Vector Operations with Scalars
 Vec<T, C> operator+(T v);
 Vec<T, C> operator-(T v);
 Vec<T, C> operator*(T v);
 Vec<T, C> operator/(T v);
 Vec<T, C>& operator+=(T v);
 Vec<T, C>& operator-=(T v);
 Vec<T, C>& operator*=(T v);
 Vec<T, C>& operator/=(T v);
 // Vector Operations with Vectors
 Vec<T, C> operator+(const Vec<T, C>& v);
 Vec<T, C> operator-(const Vec<T, C>& v);
 Vec<T, C> operator*(const Vec<T, C>& v);
 Vec<T, C> operator/(const Vec<T, C>& v);
private:
 // Recursive pusher (used by constructor)
 template<typename ... Args>
 void push(T v, Args&& ... args);
 // Base pusher
 void push(T v);
 // Vector values
 dataType values[C];
 // Index for Vector pusher
 unsigned int idx;
};
template<typename T, unsigned int C>
Vec<T, C>::Vec() {
 for ( unsigned int i = 0; i < C; ++i )
 this->values[i] = 0;
}
template<typename T, unsigned int C>
Vec<T, C>::Vec(T v) {
 for ( unsigned int i = 0; i < C; ++i )
 this->values[i] = v;
}
template<typename T, unsigned int C>
Vec<T, C>::Vec(const std::vector<T>& v) {
 VEC_ASSERT(v.size() <= C);
 for ( unsigned i = 0; i < v.size(); ++i )
 this->values[i] = v[i];
}
template<typename T, unsigned int C>
Vec<T, C>::Vec(const std::initializer_list<T>& l) {
 VEC_ASSERT(l.size() <= C);
 unsigned i = 0;
 for ( auto it : l )
 this->values[i++] = it;
}
template<typename T, unsigned int C>
template<typename ... Args>
Vec<T, C>::Vec(T v, Args&& ... args) {
 this->idx = 0;
 this->values[idx] = v;
 this->push(args ...);
}
template<typename T, unsigned int C>
template<typename ... Args>
void Vec<T, C>::push(T v, Args&& ... args) {
 this->values[++(this->idx)] = v;
 this->push(args ...);
}
template<typename T, unsigned int C>
void Vec<T, C>::push(T v) {
 VEC_ASSERT(this->idx + 1 < C);
 this->values[++(this->idx)] = v;
}
template<typename T, unsigned int C>
unsigned int Vec<T, C>::dim() const {
 return C;
}
template<typename T, unsigned int C>
double Vec<T, C>::length() const {
 double result = 0;
 for ( unsigned int i = 0; i < C; ++i )
 result += this->values[i] * this->values[i];
 return std::sqrt(result);
}
template<typename T, unsigned int C>
double Vec<T, C>::dist(const Vec<T, C>& v) const {
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] - v[i];
 return result.length();
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::cross(const Vec<T, C>& v) const {
 VEC_ASSERT(C == 3);
 Vec<T, C> result;
 result[0] = this->values[1] * v[2] - this->values[2] * v[1];
 result[1] = this->values[0] * v[2] - this->values[2] * v[0];
 result[2] = this->values[0] * v[0] - this->values[1] * v[0];
 return result;
}
template<typename T, unsigned int C>
double Vec<T, C>::dot(const Vec<T, C>& v) const {
 double result = 0.0;
 for ( unsigned int i = 0; i < C; ++i )
 result += this->values[i] * v[i];
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::ortho() const {
 VEC_ASSERT(C == 2);
 return Vec<T, C>(this->values[1], -(this->values[0]));
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::norm() const {
 VEC_ASSERT(this->length() != 0);
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] * (1.0 / this->length());
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::rotate(double angle) const {
 VEC_ASSERT(C == 2);
 double theta = angle / 180.0 * M_PI;
 double c = std::cos(theta);
 double s = std::sin(theta);
 double x = this->values[0] * c - this->values[1] * s;
 double y = this->values[0] * s + this->values[1] * c;
 return Vec<T, C>(x, y);
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::rotateX(double angle) const {
 VEC_ASSERT(C == 3);
 double theta = angle / 180.0 * M_PI;
 double c = std::cos(theta);
 double s = std::sin(theta);
 double x = this->values[0];
 double y = this->values[1] * c - this->values[2] * s;
 double z = this->values[1] * s + this->values[2] * c;
 return Vec<T, C>(x, y, z);
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::rotateY(double angle) const {
 VEC_ASSERT(C == 3);
 double theta = angle / 180.0 * M_PI;
 double c = std::cos(theta);
 double s = std::sin(theta);
 double x = this->values[0] * c + this->values[2] * s;
 double y = this->values[1];
 double z = -(this->values[0]) * s + this->values[2] * c;
 return Vec<T, C>(x, y, z);
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::rotateZ(double angle) const {
 VEC_ASSERT(C == 3);
 double theta = angle / 180.0 * M_PI;
 double c = std::cos(theta);
 double s = std::sin(theta);
 double x = this->values[0] * c - this->values[1] * s;
 double y = this->values[0] * s + this->values[1] * c;
 double z = this->values[2];
 return Vec<T, C>(x, y, z);
}
template<typename T, unsigned int C>
auto Vec<T, C>::to_std_vector() const -> std::vector<dataType> {
 return std::vector<dataType>(&this->values[0], &this->values[0] + C);
}
template<typename T, unsigned int C>
template<typename TT, unsigned int CC>
Vec<TT, CC> Vec<T, C>::to() const {
 Vec<TT, CC> result;
 for ( unsigned int i = 0; i < std::min(C, CC); ++i )
 result[i] = static_cast<TT>(this->values[i]);
 return result;
}
template<typename T, unsigned int C>
auto Vec<T, C>::operator[](int index) -> dataType_ref {
 VEC_ASSERT(index < C);
 return this->values[index];
}
template<typename T, unsigned int C>
auto Vec<T, C>::operator()(int index) -> dataType_ref {
 VEC_ASSERT(index < C);
 return this->values[index];
}
template<typename T, unsigned int C>
auto Vec<T, C>::operator[](int index) const -> dataType_cref {
 VEC_ASSERT(index < C);
 return this->values[index];
}
template<typename T, unsigned int C>
auto Vec<T, C>::operator()(int index) const -> dataType_cref {
 VEC_ASSERT(index < C);
 return this->values[index];
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator+(T v) {
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] + v;
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator-(T v) {
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] - v;
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator*(T v) {
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] * v;
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator/(T v) {
 VEC_ASSERT(v != 0);
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] / v;
 return result;
}
template<typename T, unsigned int C>
Vec<T, C>& Vec<T, C>::operator+=(T v) {
 for ( unsigned int i = 0; i < C; ++i )
 this->values[i] += v;
 return *this;
}
template<typename T, unsigned int C>
Vec<T, C>& Vec<T, C>::operator-=(T v) {
 for ( unsigned int i = 0; i < C; ++i )
 this->values[i] -= v;
 return *this;
}
template<typename T, unsigned int C>
Vec<T, C>& Vec<T, C>::operator*=(T v) {
 for ( unsigned int i = 0; i < C; ++i )
 this->values[i] *= v;
 return *this;
}
template<typename T, unsigned int C>
Vec<T, C>& Vec<T, C>::operator/=(T v) {
 VEC_ASSERT(v != 0);
 for ( unsigned int i = 0; i < C; ++i )
 this->values[i] /= v;
 return *this;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator+(const Vec<T, C>& v) {
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] + v[i];
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator-(const Vec<T, C>& v) {
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] - v[i];
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator*(const Vec<T, C>& v) {
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] * v[i];
 return result;
}
template<typename T, unsigned int C>
Vec<T, C> Vec<T, C>::operator/(const Vec<T, C>& v) {
 for ( unsigned int i = 0; i < C; ++i )
 VEC_ASSERT(v[i] != 0);
 Vec<T, C> result;
 for ( unsigned int i = 0; i < C; ++i )
 result[i] = this->values[i] / v[i];
 return result;
}
typedef Vec<int, 2> Vec2i;
typedef Vec<int, 3> Vec3i;
typedef Vec<int, 4> Vec4i;
typedef Vec<unsigned int, 2> Vec2u;
typedef Vec<unsigned int, 3> Vec3u;
typedef Vec<unsigned int, 4> Vec4u;
typedef Vec<float, 2> Vec2f;
typedef Vec<float, 3> Vec3f;
typedef Vec<float, 4> Vec4f;
typedef Vec<double, 2> Vec2d;
typedef Vec<double, 3> Vec3d;
typedef Vec<double, 4> Vec4d;
typedef Vec<char, 2> Vec2c;
typedef Vec<char, 3> Vec3c;
typedef Vec<char, 4> Vec4c;
typedef Vec<unsigned char, 2> Vec2uc;
typedef Vec<unsigned char, 3> Vec3uc;
typedef Vec<unsigned char, 4> Vec4uc;
Sᴀᴍ Onᴇᴌᴀ
29.6k16 gold badges45 silver badges203 bronze badges
asked Aug 23, 2018 at 21:58
\$\endgroup\$
6
  • \$\begingroup\$ Welcome to Code Review! I rolled back your edits. After getting an answer you are not allowed to change your code anymore. This is to ensure that answers do not get invalidated and have to hit a moving target. If you have changed your code you can either post it as an answer (if it would constitute a code review) or ask a new question with your changed code (linking back to this one as reference). Refer to this post for more information \$\endgroup\$ Commented Aug 24, 2018 at 19:49
  • \$\begingroup\$ @SamOnela Sure, I will change it back. \$\endgroup\$ Commented Aug 24, 2018 at 19:51
  • \$\begingroup\$ I already did that. \$\endgroup\$ Commented Aug 24, 2018 at 19:51
  • \$\begingroup\$ @SamOnela Okay. I just thought I should include what rsy56640 suggested. \$\endgroup\$ Commented Aug 24, 2018 at 19:52
  • \$\begingroup\$ Which C++ version are you using? Can I assume C++17? \$\endgroup\$ Commented Aug 25, 2018 at 9:29

2 Answers 2

3
\$\begingroup\$

Finally a real vector class :P.

  1. Any reason why you use VEC_ASSERT instead of just assert. I don't really see the advantage of doing so.

  2. using declarations are nicer than typedefs IMO:

    using dataType = T;
    
  3. If you use exceptions, mark functions that don't or shouldn't throw noexcept.

  4. Make use of the injected-class-name:

    Vec cross(const Vec &v) const;
    
  5. You should implement all the @= operators for vectors. Also unary -.

  6. Use standard algorithms:

    std::fill(values, values + C, 0); // default constructor, second one too
    std::copy(v.begin(), v.end(), values); // std::vector constructor, init list
    std::inner_product(v.begin(), v.end(), v.begin(), 0); // length/dot
    std::transform(values, values + C, result.values, [length](const auto& value) {
     return values / length;
    }); // norm, operator@
    assert(std::all_of(v.values, v.values + C, [](const auto& value) {
     return value != 0;
    })); // operator/
    
  7. You don't have to use this-> everywhere you know :).

  8. You should consider using assert messages: assert(v[0] == 0 && "the first element must be 0!");

  9. No love for long double and signed char? They don't have aliases.

  10. You don't fill the rest of the elements to 0 in your std::vector, initializer list constructors and in to.

  11. You don't need push by changing your signature a bit and doing:

    template <typename T, unsigned int C>
    template <typename... Args>
    Vec<T, C>::Vec(Args&&... args) : Vec{std::forward<Args>(args)...} {
     static_assert(sizeof...(Args) <= C, "too many arguments to vector");
    }
    
  12. There are techniques to avoid implementing similar code between operator@ and operator@=:

    friend Foo operator+(Foo lhs, const Foo& rhs) {
     lhs += rhs;
     return lhs;
    }
    Foo& operator+=(const Foo& rhs) const {
     // do logic
     return *this;
    }
    

    More information very good advice for overloading operator can be found on cppref.

  13. If you want you can reduce the code duplicate of cv qualified operator[]s by using const_cast.

  14. Please prohibit creating a vector of length 0! :)

  15. You could provide rvalue overloads of operator[] to enable efficient moving, but you don't have to. This is overkill, mostly used in the standard library. Have a look at std::optional::operator* to see this in action.

  16. You might want to consider adding a constructor that takes a pair of iterators, so that Vec can be initialized by anything really, not just a std::vector.

  17. Consider making everything constexpr.

answered Aug 25, 2018 at 10:22
\$\endgroup\$
16
  • \$\begingroup\$ Thanks a lot for all your suggestions. 1) No, there was something else I wanted to do and I didn't, unfortunately I can't re-post the code. 2) I could use using, 3) yes, I will use exceptions, 4) does this work for the implementation as well? 5) if you mean ==, !=, <, > etc, I will. I am just thinking 2 possible ways: a) == returns true or false if both vectors are the same or not and b) == returns a vector of bools (like matlab does) 6) yes, I should and I will, 7) I know, it's just my style, 8) I am still thinking if I should go with assert or exceptions or both (any suggestion)? \$\endgroup\$ Commented Aug 25, 2018 at 11:12
  • \$\begingroup\$ @DimChtz 5) Oh yeah that too. I meant Vec& operator+=(const Vec&) \$\endgroup\$ Commented Aug 25, 2018 at 11:14
  • \$\begingroup\$ 9) Yes, eventually I will add more aliases, 10) True, I will fix this, 11) Nice, I didn't know this, 12) will do that too, 13) i didn't think of that :P, 14) I was thinking of that, should I use enable_if?, 15) I think I lost you here :P, 16) I will \$\endgroup\$ Commented Aug 25, 2018 at 11:15
  • \$\begingroup\$ @DimChz Use asserts for precondition violation. I'd use asserts in your case, because they should be guaranteed by the caller. \$\endgroup\$ Commented Aug 25, 2018 at 11:15
  • \$\begingroup\$ Yes, I will eventually include everything. I am still doing changes \$\endgroup\$ Commented Aug 25, 2018 at 11:16
2
\$\begingroup\$
template<typename T, unsigned int C>
class Vec;

The template generates redundant codes if C varies for the same T.
You can look through <<Effective C++>> Item 44 and refactor like this:

template<typename T>
class VecBase {
 T* data;
public:
 VecBase(std::size_t C);
};
template<typename T, unsigned int C>
class Vec :VecBase<T> {
public:
 Vec() :VecBase<T>(C)
 {}
};

The benefit is:

  • prevent software from being too large.
  • shrink the size of Vec<T, C> (data is allocated on the heap, just contains a pointer)
Rakete1111
2,5821 gold badge16 silver badges23 bronze badges
answered Aug 24, 2018 at 10:28
\$\endgroup\$
4
  • \$\begingroup\$ Thanks for your answer. Should I use VecBase only for storing the data and keep everything else inside Vec? \$\endgroup\$ Commented Aug 24, 2018 at 14:44
  • \$\begingroup\$ Right, and remember to correctly implement copy/move/dtor. \$\endgroup\$ Commented Aug 24, 2018 at 15:06
  • 1
    \$\begingroup\$ I think this: Vec() :VecBase(C) should be: Vec() : VecBase<T>(C). \$\endgroup\$ Commented Aug 24, 2018 at 18:35
  • 1
    \$\begingroup\$ Please don't use the term heap it is not meaningful in a C++ context (Don't be fooled because Java uses these terms). There is no reference to heap/stack in the C++ standard for a good reason. The terms you should be using are automatic/static/dynamic storage duration objects. Here the difference is between automatic and dynamic objects. \$\endgroup\$ Commented Aug 28, 2018 at 6:25

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.