C++ simple 2D vector

Question

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;
 }
 // add op
 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;
}

Answer 1

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.

• Alexis Drakopoulos: 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.
• JDługosz: 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.
• ades: You're right of course @JDługosz, don't know what I was thinking there.
• Cris Luengo: I don’t see a reason to make x and y private. There is no class invariant to maintain, this no need to prevent the user from randomly changing their values. Making them private reduces the usefulness of the class.

Answer 2

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';
}

• ades: I think the recommendation to use a print of \n over std::endl merits a bit more explanation - is flushing always bad?
• Alexis Drakopoulos: No worries @ades the flushing part I don't mind for this example. this is very useful thank you
• Alexis Drakopoulos: Could you provide a link/source for how that equality 1 liner works, I have no understanding of why we can simply do const = default
• Alexis Drakopoulos: This doesn't compile on clion due to there not being a constructor, why is not using the default constructor?
• Toby Speight: @AlexisDrakopoulos, have you set your compiler to accept a sufficiently modern version of C++? It's a good idea to select the most recent version it can support; e.g. I generally use g++ -std=c++20 when compiling with GCC.