4
\$\begingroup\$

I have rewritten the Vector2 class that I usually use in my projects. The code looks as follows:

#pragma once

#include <type_traits>
#include <utility>
#include <iostream>

namespace ae
{
    // Does T support constexpr?
    template <Literal T>
    concept Literal = std::is_fundamental_v<T>;

    template <Literal T>
    class Vector2
    {
    public:
        constexpr Vector2() noexcept :
        x(T{}),
        y(T{})
        {}

        constexpr Vector2(T x, T y) noexcept :
        x(x),
        y(y)
        {}

        constexpr Vector2(const Vector2<T>& other) noexcept :
        x(other.x),
        y(other.y)
        {}

        constexpr Vector2(const Vector2<T>&& other) noexcept :
        x(std::move(other.x)),
        y(std::move(other.y))
        {}

        constexpr void operator =(const Vector2<T>& rhs) noexcept
        {
            this->x = rhs.x;
            this->y = rhs.y;
        }

        template <Literal T>
        constexpr void operator =(const Vector2<T>&& rhs) noexcept
        {
            this->x = std::move(rhs.x);
            this->y = std::move(rhs.y);
        }

        T x;
        T y;
    };

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T> operator -(const Vector2<T>& rhs) noexcept
    {
        return { -rhs.x, -rhs.y };
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T>& operator +=(Vector2<T>& lhs, const Vector2<T>& rhs) noexcept
    {
        lhs.x += rhs.x;
        lhs.y += rhs.y;

        return lhs;
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T>& operator -=(Vector2<T>& lhs, const Vector2<T>& rhs) noexcept
    {
        lhs.x -= rhs.x;
        lhs.y -= rhs.y;

        return lhs;
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T> operator +(const Vector2<T>& lhs, const Vector2<T>& rhs) noexcept
    {
        return { lhs.x + rhs.x, lhs.y + rhs.y };
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T> operator -(const Vector2<T>& lhs, const Vector2<T>& rhs) noexcept
    {
        return { lhs.x - rhs.x, lhs.y - rhs.y };
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T> operator *(const Vector2<T>& lhs, T rhs) noexcept
    {
        return { lhs.x * rhs, lhs.y * rhs };
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T> operator *(T lhs, const Vector2<T>& rhs) noexcept
    {
        return { rhs.x * lhs, rhs.y * lhs };
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T>& operator *=(Vector2<T>& lhs, T rhs) noexcept
    {
        lhs.x *= rhs;
        lhs.y *= rhs;

        return lhs;
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T> operator /(const Vector2<T>& lhs, T rhs) noexcept
    {
        return { lhs.x / rhs, lhs.y / rhs };
    }

    template <Literal T>
    [[nodiscard]] constexpr Vector2<T>& operator /=(Vector2<T>& lhs, T rhs) noexcept
    {
        lhs.x /= rhs;
        lhs.y /= rhs;

        return lhs;
    }

    template <Literal T>
    [[nodiscard]] constexpr bool operator ==(const Vector2<T>& lhs, const Vector2<T>& rhs) noexcept
    {
        return (lhs.x == rhs.x) && (lhs.y == rhs.y);
    }

    template <Literal T>
    [[nodiscard]] constexpr bool operator !=(const Vector2<T>& lhs, const Vector2<T>& rhs) noexcept
    {
        return (lhs.x != rhs.x) || (lhs.y != rhs.y);
    }

    template <Literal T>
    std::ostream& operator <<(std::ostream& os, const Vector2<T>& rhs) noexcept
    {
        return os << "{" << rhs.x << ", " << rhs.y << "}";
    }
}

Looking for any feedback, perhaps especially style wise. Note that the class does not represent a 'mathematical vector' just a 2d coordinate / pair.

I have a Vector3 and Vector4 class as well, so any improvements that apply here, should also apply there.

\$\endgroup\$
2
  • \$\begingroup\$ Why do you have [[nodiscard]] on your compound assignment operators (+=, -=, etc.)? Those are often used for the side effect and ignore the returned value. \$\endgroup\$ May 10 '20 at 21:19
  • \$\begingroup\$ That is a mistake. I copy pasted the declartions and edited the names afterwards. Thank you for pointing it out :P. \$\endgroup\$
    – Cortex
    May 10 '20 at 21:31
2
\$\begingroup\$
// Does T support constexpr?
template <Literal T>
concept Literal = std::is_fundamental_v<T>;

I see what you're going for. This isn't bad, but imagine someone tries to make a Vector2 with a custom type... they'll have to find this comment to understand why compilation failed. If you just use typename T and get rid of the concept, then the compiler will show a traceback and say "T is not constexpr" which is a lot more helpful to the user. TL;DR: I'd get rid of this concept.


This is a lot of constructors. Is there any reason not to use the default ones?


Can you leverage std::pair to define the constructors/the comparison operators for you? Maybe you can inherit from it?


constexpr Vector2<T> operator *(const Vector2<T>& lhs, T rhs)

This has the same semantics as converting rhs into Vector2<T>(rhs, rhs) ... maybe it makes sense to have a constructor from a single T to a Vector2 with the same T in each dimension?

This would also allow you to define a single operator* and rely on implicit conversion for scalar values.


Overall this looks pretty good.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.