Comment on the style of this C++ class for a 2D Vector

Question

I wrote a template class for a 2D vector that is intended to provide 2D vector arithmetic to be used in a Physics simulation program. Note that x and y are public or ease of use since encapsulation here would just decrease the readability of code that uses the class.

Is the general style of my code good and does it comply with modern C++ best practices? Thank you for the suggestions.

template <typename T>
class Vector2D final {
public:
    T x;
    T y;

    // Defaulted constructors
    Vector2D(const Vector2D<T>& v) = default;
    ~Vector2D() = default;

    Vector2D(T ix, T iy);

    // Member Utility Functions
    void Set(T ix, T iy);
    T Magnitude();

    // Overloaded Compound Operators
    Vector2D<T>& operator+=(const Vector2D<T>& rhs);
    Vector2D<T>& operator*=(const Vector2D<T>& rhs);
    Vector2D<T>& operator-=(const Vector2D<T>& rhs);
    Vector2D<T>& operator/=(const T& rhs);
    Vector2D<T>& operator=(const  Vector2D<T>& rhs);
};

// Non-Compound Operators.
template <typename T> Vector2D<T> operator+(Vector2D<T> lhs, const Vector2D<T>& rhs);
template <typename T> Vector2D<T> operator*(Vector2D<T> lhs, const Vector2D<T>& rhs);
template <typename T> Vector2D<T> operator-(Vector2D<T> lhs, const Vector2D<T>& rhs);
template <typename T> Vector2D<T> operator/(Vector2D<T> lhs, const T& rhs);

template <typename T>
Vector2D<T>::Vector2D(T ix, T iy)
        : x(ix)
        , y(iy)
{
}

template <typename T>
void Vector2D<T>::Set(T ix, T iy) {
    x = ix;
    y = iy;
}

template <typename T>
T Vector2D<T>::Magnitude() {
    return x*x + y*y;
}

template <typename T>
Vector2D<T>& Vector2D<T>::operator+=(const Vector2D<T>& rhs) {
    x += rhs.x;
    y += rhs.y;
    return *this;
}

template <typename T>
Vector2D<T>& Vector2D<T>::operator*=(const Vector2D<T>& rhs) {
    x *= rhs.x;
    y *= rhs.y;
    return *this;
}

template <typename T>
Vector2D<T>& Vector2D<T>::operator-=(const Vector2D<T>& rhs) {
    x -= rhs.x;
    y -= rhs.y;
    return *this;
}

template <typename T>
Vector2D<T>& Vector2D<T>::operator/=(const T& rhs) {
    x /= rhs;
    y /= rhs;
    return *this;
}

template <typename T>
Vector2D<T>& Vector2D<T>::operator=(const Vector2D<T>& rhs) {
    x = rhs.x;
    y = rhs.y;
    return *this;
}

template <typename T>
Vector2D<T> operator+(Vector2D<T> lhs, const Vector2D<T>& rhs) {
    lhs += rhs;
    return lhs;
}

template <typename T>
Vector2D<T> operator*(Vector2D<T> lhs, const Vector2D<T>& rhs) {
    lhs *= rhs;
    return lhs;
}

template <typename T>
Vector2D<T> operator-(Vector2D<T> lhs, const Vector2D<T>& rhs) {
    lhs -= rhs;
    return lhs;
}

template <typename T>
Vector2D<T> operator/(Vector2D<T> lhs, const T& rhs) {
    lhs /= rhs;
    return lhs;
}

Some examples of using this with float. I intend to only use it with floating point types (float, double, long double etc) and have tested it with other types but will present float here for brevity.

Vector2D<float> v1 = {1.0, 2.0};
Vector2D<float> v2 = {2.0, 3.0};

v1 += v2;
assert(v1.x == 3.0 && v1.y == 5.0);
v1 -= v2;
assert(v1.x == 1.0 && v1.y == 2.0);
v1 *= v2;
assert(v1.x == 2.0 && v1.y == 6.0);
v1 /= 2.0;
assert(v1.x == 1.0 && v1.y == 3.0);

v1.Set(1.0, 2.0);
v2.Set(2.0, 3.0);
Vector2D<float> v3 = {0.0, 0.0};

v3 = v1 + v2;
assert(v3.x == 3.0 && v3.y == 5.0);
v3 = v1 - v2;
assert(v3.x == -1.0 && v3.y == -1.0);
v3 = v1 * v2;
assert(v3.x == 2.0 && v3.y == 6.0);
v3 = v1 / 2.0f;
assert(v3.x == 0.5 && v3.y == 1.0);

Answer 1

This looks pretty good! I don't think you need to change much, but there are some things missing that you will likely need:

1. normalize() or normal() method to either normalize a vector or return a normalized version of the vector. If you're doing physics, you'll need normalized vectors for lots of operations, probably.
2. Scalar multiply (operator*(const T)). You'll likely need to scale vectors frequently and will do that by multiplying by a scalar.
3. operator==() and/or operator!=(). You should define one in terms of the other.
4. Possibly a squared magnitude. Sometimes taking the square root in the Magnitude() function is a bottleneck and you don't need it (for example to see if a point is within a given radius of another point). It can be faster to get the squared magnitude of the vector between the two points if that's something you do a lot.

Answer 2

I see a few things that might be improved with this code.

Provide a constexpr constructor

Right now, this line fails to compile:

constexpr Vector2D<float> unit{1,1};

This is easily solved by simply adding the keyword constexpr to the constructor.

Consider using std::enable_if

These lines currently compile just fine:

Vector2D<std::string> vstring{"nine", "seven"};
Vector2D<std::complex<float>> vc{ {3,4}, {5,12} };

Should they? If your intent was to only support arithmetic types, then you might want to make that explicit by using std::enable_if and std::is_arithmetic.

Consider creating a default constructor

Some things are made easier if there is a default constructor. For example, neither of these will compile right now:

std::vector<Vector2D<float>> v;  
std::vector<Vector2D<float>> v(10);

We can still create a vector of Vector2D but we have to supply our own initalizer:

std::vector<Vector2D<float>> v(10, {3,4});

Do you really need Set?

In the search for a minimally sufficient interface, I'd suggest that Set is probably not really needed since the data members are public.