# 3D mathematical vector class

I've been working on a 3D mathematical vector class which should be as streamlined as possible for use in numerical simulations. It will be used to model 3D-physical vectors.

Here, 3D-vector should be taken in mathematical sense, meaning a tuple (a,b,c).

I hoped to design it in a modern and fast way - but one is never perfect. So, I would be interested in some input from your side. Any tips for making this faster?

//threevector.h
#ifndef threevector_h_
#define threevector_h_

#include <fstream>
#include <cmath>
#include <array>

template <class T>
class threevector
{
private:
static const int dim = 3; //dimension of vector
std::array<T,dim> container;

public:

//constructors and assignment
threevector(const double a = 0, const double b = 0, const double c = 0):
container({{a,b,c}}) {}; //standard constructor
threevector(const threevector& a): container(a.container) {};
//copy constructor

// add once gcc 4.7 is used
//   threevector(threevector&& other): threevector() {swap(*this, other);}
// move constructor

threevector& operator=(threevector rhs) //assignment
{
swap(*this, rhs);
return *this;
}

void swap(threevector& first, threevector& second)
{first.container.swap(second.container);}

//operators
threevector& operator+=(const threevector& rhs)
{
container += rhs.container;
container += rhs.container;
container += rhs.container;
return *this;
}

threevector& operator-=(const threevector& rhs)
{
*this += -rhs;
return *this;
}

threevector& operator*=(const double rhs) //scalar multiplication assignment
{
container *= rhs;
container *= rhs;
container *= rhs;
return *this;
}

threevector& operator/=(const double rhs) //scalar division assignment
{
*this *= 1./rhs;
return *this;
}

threevector operator+() const //unary plus
{
threevector a(*this);
return a;
}

threevector operator-() const //unary minus
{
threevector a(*this);
a *= -1;
return a;
}

T& operator[](const int input) {return container[input];} //access operator

const T& operator[](const int input) const {return container[input];}
// const access operator

//utility functions
double abs() const
{return sqrt(container*container+container*container+
container*container);}
double abs_sq() const {return pow(abs(),2);}
void reset() { container = 0; container = 0; container = 0;}
};

//output operator
template<class T>
std::ostream& operator<<(std::ostream& os, const threevector<T>& obj)
{
os << std::fixed << "(" << obj << "," << obj << "," << obj << ")";
return os;
}

template<class T>
inline threevector<T> operator+(threevector<T> lhs, const threevector<T>& rhs)
{
lhs += rhs;
return lhs;
}

//subtraction operator
template<class T>
inline threevector<T> operator-(threevector<T> lhs, const threevector<T>& rhs)
{
lhs -= rhs;
return lhs;
}

//scalar product
template<class T>
inline double operator*(const threevector<T>& lhs, const threevector<T>& rhs)
{return lhs * rhs + lhs * rhs + lhs * rhs;}

//product with scalar
template<class T>
inline threevector<T> operator*(const double lhs, threevector<T> rhs)
{
rhs *= lhs;
rhs *= lhs;
rhs *= lhs;
return rhs;
}

//product with scalar
template<class T>
inline threevector<T> operator*(threevector<T> lhs, const double rhs){return rhs*lhs;}

// scalar division
template<class T>
inline threevector<T> operator/(threevector<T> lhs, const double rhs){return lhs*(1./rhs);};

#endif


Your design is a little bit confusing. You define your class as a template class, but just about everything takes and returns a double. Generally, you should try and be consistent: will this class work for any numeric-like T? Then make sure it has constructors that take a T instead of a double:

threevector(T a = T(), T b = T(), T c = T())


Similarly for the overloaded operators that multiply/divide by a scalar, and so on.

The name threevector is awkward. At the least it should be three_vector, but that's still a little awkward. vector_3d is more of the "standard" name for this kind of thing.

Your include guards should generally be in all-caps. This is because they are global in scope, and you want to do as much as possible to minimize the possibility of potential name clashes.

#ifndef threevector_h_ // should be THREE_VECTOR_H_
#define threevector_h_ // or VECTOR_3D_H_


You have some semi-colons where they aren't needed:

threevector(const double a = 0, const double b = 0, const double c = 0):
container({{a,b,c}}) {}; //standard constructor
threevector(const threevector& a): container(a.container) {}; // <--- Not needed


Comments like //standard constructor are obvious and don't really need to be there. Further, your copy constructor and copy assignment operator are redundant; the compiler generated ones are sufficient.

It's often easiest to write operator+ (and -, * etc) in terms of operator+=:

vector_3d operator+(const vector_3d& a, const vector_3d& b)
{
vector_3d result(a);
result += b;
return result;
}


For something this simple, it's unlikely you'll be able to speed it up much. The only potential that I can see is replacing the call to pow() in:

double abs_sq() const {return pow(abs(),2);}


with something like:

T abs_sq() const
{
const T ab = abs();  // abs() should also return a T
return ab * ab;
}


which avoids the overhead of a function call to pow. In practice, this is unlikely to make much of a difference, however.

• "Further, your copy constructor and copy assignment operator are redundant; the compiler generated ones are sufficient." Are you sure that the compiler knows about copy-and-swap? – user3058865 Mar 14 '14 at 9:46
• In this case, it doesn't need to. You only really need to use copy-and-swap with a resource managing class. Your class doesn't manage any resources (that is, anything that is directly newed), hence copy-and-swap is redundant. – Yuushi Mar 14 '14 at 12:23
• Many of these comments are pointless. They should only be needed if something is not obvious and needs explanation. There's no need for individual comments for each constructor and operator; these are already noticeable to those familiar with C++.

• There's no need for inline here. Code in headers are automatically inline, so this would apply to all the code here. Having it here anyway wouldn't cause a problem as the compiler could just remove it, but I'd remove them anyway.

• dim could just be renamed to dimensions, so that you won't need a comment for it:

static const int dimensions = 3;

• This is not too readable:

void swap(threevector& first, threevector& second)
{first.container.swap(second.container);}


This would also reduce maintainability, especially if additional lines will need to be added. Using your existing curly brace style, you'll have this:

void swap(threevector& first, threevector& second)
{
first.container.swap(second.container);
}


I'd also recommend keeping this consistent throughout the code. You already use it some places (with the multi-line functions), but this should also be done with the single-line functions.

• For statements like these:

threevector& operator-=(const threevector& rhs)
{
*this += -rhs;
return *this;
}


you can make it a single line:

threevector& operator-=(const threevector& rhs)
{
return *this += -rhs;
}

• I'd use some parenthesis in the scalar product for clarity:

{
return (lhs * rhs) + (lhs * rhs) + (lhs * rhs);
}