Attempt at templates by creating a class for N-dimensional mathematical vectors

Originally I had tried to implement this topic after having learned about Inheritence and posted it on here, but that forced complexity and the main suggestion was that this was ideal for templates. Now that I've learnt about templates I was hoping someone could review my code for a basic implementation of mathematical vectors.

One thing I will note: There probably is a way to generalise the cross-product to N-dimensions but I wasn't too interested in finding out how since that's a more mathematical query, I just wanted to code so I only have cross product for three dimensions.

My main concerns are the uses of headers and source files, is it okay to have all of the implementation in one header file and the few necessary non-template functions defined in source files (like cross_product)?

And how should I treat friendship for cross_product? It only needs to be friends with Vector, but I couldn't see a way of making exclusive Vector friendship without using template specialisation, which seemed a bit too much. So it is friends with all Vector but only accesses members of Vectors.

Inside Vector.h:

```
#ifndef MATHSVectors
#define MATHSVectors

#include
#include
#include
#include //for the exceptions
#include //sqrt, sin, cos, abs

//main template forward declaration
template class Vector;
template std::ostream& operator&);
template std::istream& operator>> (std::istream&, Vector&);
template bool operator==(const Vector&, const Vector&);
template bool operator!=(const Vector&, const Vector&);
template Vector operator+(const Vector&, const Vector&);
template Vector operator-(const Vector&, const Vector&);
template Vector operator*(const Vector&, double);
template Vector operator*(double, const Vector&);
template Vector operator/(const Vector&, double);
template double dot_product(const Vector&, const Vector&);

//in Vector.cpp
bool double_equals(double, double);
double approximate(double, double);
Vector cross_product(const Vector&,

This looks pretty good. I don't have any deal-breaker comments, just a bunch of minor ones.

Prefer std::array

Just straight up std::array x instead of double x[N]. Raw arrays are broken. There's not really any real difference for the purposes of your use-case, but it's just a more pleasant type to deal with in general.

Throwing and non-throwing

For functions that throw, there's an extra mechanism that needs to exist to support that use-case. Plus even an always-predicted branch is going to be more code that no branch. To that end the standard containers offer throwing and non-throwing functions. If you rewrite your two operator[]s to be non-throwing:

template 
double& Vector::operator[](size_t p) {
    return x[p];
}

And introduce a throwing alternative:

template 
double& Vector::at(size_t p) {
    if(p >= N) throw std::out_of_range(std::string("Invalid coordinate specified for function ") + __func__); 
    return (*this)[p];
}

That will give you two benefits. First, users of your class would expect operator[] to not throw since that's pretty typical. But second, you no longer need to friend any of the free functions. cross_product only needed to be a friend because you wanted to avoid the throwing, now that's just a non-issue.

Use some standard algorithms

Your equality operator can be reimplemented with std::equal:

template  
bool operator==(const Vector& lhs, const Vector& rhs) {
    return std::equal(lhs.x.begin(), lhs.x.end(), rhs.x.begin(), double_equals);
}

Similarly, operator bool doesn't need to construct a whole new vector, just check the one you have against zero with std::any_of:

template 
Vector::operator bool() const {
    return std::any_of(lhs.x.begin(), lhs.x.end(), [](double v){
        return !double_equals(v, 0.0);
    });
}

Initialization from std::initializer_list can use std::copy:

std::copy(x.begin(), x.end(), li.begin());

Normalize

I suspect the typical case here might be for valid vectors, so prefer to take the length first and compare that against zero, rather than invoking operator!:

double magnitude = length();
if (magnitude > 0) {
    (*this) /= magnitude;
}
return *this;

I also question the name length(). That seems to suggest that this is a container more along the lines of std::vector, but really you're talking about the norm of the vector. So a better name, please.

Use range-based for

Whenever you loop in a way that doesn't actually involve the index, prefer to use range-based for. It's just easier to write. For instance, your operator*=:

for (double& val : x) {
    val *= d;
}
return *this;

Friend Operators

Rather than forward declaring template functions, then friending them, prefer to write these operators as non-member non-template friends. So operator+ would be:

template 
class Vector {
public:
    friend Vector operator+(Vector const& lhs, Vector const& rhs) {
        Vector sum = lhs;
        sum += rhs;
        return sum;
    }
};

This will be found by ADL and nothing else, and then you also don't have to worry about some of the other issues that writing function templates leads you to. It's just simpler.

rotateCoordinates

This seems like it should be a non-member to me.

StackExchange Code Review Q#111746, answer score: 5