C++ simple 2D vector

I'm brand new to cpp and tried implementing a simple Vector class that supports abs(), equality and addition operators. I just wanted a quick set of eyes to tell me if I'm doing anything egregious in my constructor/operators and finally their usage.

Note: the [[nodiscard]] was recommended by the clion IDE.

#include <iostream>
#include <cmath>

class Vector {
public:
double x, y;
[[nodiscard]] double abs() const {return sqrt(x*x + y*y);}

// constructor
Vector(double x, double y) {
this->x = x;
this->y = y;
}

Vector operator+(const Vector& b) {
Vector vec(this->x, this->y);
vec.x += b.x;
vec.y += b.y;
return vec;
}

// eq op
bool operator==(const Vector& b) {
if (this->x == b.x && this->y == b.y) {
return true;
}
return false;
}
};

int main()
{
Vector vec = Vector(4, 8);
Vector vec2 = Vector(3.2, 5.5);
Vector vec3 = vec + vec2;
bool equal = vec == vec2;
bool equal2 = vec == Vector(4, 8);

std::cout << equal << std::endl;
std::cout << equal2 << std::endl;
std::cout << vec3.x << std::endl;
std::cout << vec.abs() << std::endl;
}

For the add operator, you're currently constructing a brand new vector when you do Vector vec(this->x, this->y), before manipulating the newly constructed vector's x and y values. If this is what you want to do, you can add the values in the constructor instead: return Vector(x+other.x, y+other.y). You may, however, also want to implement and use the incremental add operator +=, where you instead just could add to this->x and this->y and return *this. If you do this you should also return by reference, i.e.:

Vector& operator+=(const Vector& other) {
this->x += x;
this->y += y;
return *this;
}

You probably want your x and y members to be private and not public. And use member initialisation - your constructor can be changed to

Vector(double x, double y) : x(x), y(y) {}

Also consider that if (condition) return true else return false is the same as return condition, i.e. you can do

bool operator==(const Vector& other) {
return (this->x == other.x && this->y == other.y);
}

Finally, maybe have a look at some existing libraries that implement vectors (in 2 or 3 dimensions - shouldn't matter much). These should be able to give you some help or inspiration.

• Thanks a lot for the advice, some of it is going over my head so I'll look some things up today and get back to you. Jan 5 at 12:26
• Using member initialization (rather than assigning in the constructor's body) does not enable RAII. I don't see what RAII has to do with this class, at all. Jan 11 at 15:47
• You're right of course @JDługosz, don't know what I was thinking there.
Jan 11 at 21:08

In abs(), we use global-namespace sqrt. You probably get away with this on a platform where <cmath> also provides that, but to be truly portable, don't rely on that - use its full name std::sqrt. Better still, we can use std::hypot(), which avoids potential problems when the sum of the squares exceeds the range of double.

The [[nodiscard]] annotation seems well-intentioned, but isn't really necessary - nobody is likely to use a const member function for side-effects and fail to use the result, and no harm comes from that, making the annotation arguably just clutter. The real benefit is for functions such as std::scanf() where the return value tends to be overlooked by users.

In the constructor, we should prefer to initialise the members, rather than assign them in the body:

Vector(double x, double y)
: x{x},
y{y}
{
}

However, we don't need a constructor at all, if we're happy to have a default constructor as well. We'll want our default constructor to create a useful default value - I'd recommend {0,0} - and we can achieve that by providing default values for x and y:

double x = 0;
double y = 0;

Consider providing a += operator. We can then use that to implement + as a non-member function:

Vector& operator+=(const Vector& b) {
x += b.x;
y += b.y;
return *this;
}

// (outside the class)
Vector operator+(Vector a, const Vector& b) {
return a += b;
}

Note that we accept a by value, so we can modify our copy of it, but b can be a const reference.

The equality operator should be const, and can be defaulted in current standard C++:

bool operator==(const Vector& b) const = default;

In the main() function, we're unnecessarily flushing each line with std::endl. Plain newline (\n) is more efficient.

Modified code

#include <cmath>

struct Vector
{
double x = 0;
double y = 0;

bool operator==(const Vector& b) const = default;

Vector& operator+=(const Vector& b) {
x += b.x;
y += b.y;
return *this;
}

double abs() const
{
return std::hypot(x, y);
}
};

Vector operator+(Vector a, const Vector& b) {
return a += b;
}
#include <iomanip>
#include <iostream>

int main()
{
auto vec = Vector{4, 8};
auto vec2 = Vector{3.2, 5.5};
auto vec3 = vec + vec2;
bool equal = vec != vec2;
bool equal2 = vec == Vector{4, 8};

std::cout << std::boolalpha
<< equal << '\n'
<< equal2 << '\n'
<< vec3.x << '\n'
<< vec.abs() << '\n';
}
• I think the recommendation to use a print of \n over std::endl merits a bit more explanation - is flushing always bad?