C++ smart pointers and classes

Question

I've started learning C++ recently and want from what I've gathered smart pointers are the way to go when storing stuff on the free store. I want to make sure I've got the basic hang of C++(11) correctly. I've implemented a very basic vector class (a mathematical vector) and would greatly appreciate if anyone would point out if I'm doing something wrong or that could be done better.

Vector.h

#ifndef VECTOR
#define VECTOR

#include <memory>

class Vector {
private:
    int x;
    int y;
    int z;

public:
    Vector(int x, int y, int z);
    ~Vector();

    std::unique_ptr<Vector> operator + (Vector* vector);

    int GetX() { return x; }
    int GetY() { return y; }
    int GetZ() { return z; }

    void SetX(int x) { this->x = x; }
    void SetY(int y) { this->y = y; }
    void SetZ(int z) { this->z = z; }
};

#endif

Vector.cpp

#include <memory>
#include <iostream>
#include "Vector.h"

Vector::Vector(int x, int y, int z) : x(x), y(y), z(z) {
    std::cout << "Constructed a Vector instance with elements ( " << x << ", " << y << "," << z << " )." << std::endl;
};

Vector::~Vector() {
    std::cout << "Destructed a Vector instance with elements ( " << x << ", " << y << "," << z << " )." << std::endl;
}

std::unique_ptr<Vector> Vector::operator + (Vector* vector) {
    std::unique_ptr<Vector> result = std::make_unique<Vector>(0, 0, 0);

    result->SetX(this->x + vector->GetX());
    result->SetY(this->y + vector->GetY());
    result->SetZ(this->z + vector->GetZ());

    return result;
};

Usage

#include <iostream>
#include <memory>
#include "Vector.h"

int main() {
    std::unique_ptr<Vector> v1 = std::make_unique<Vector>(1, 2, 3);
    std::unique_ptr<Vector> v2 = std::make_unique<Vector>(3, 2, 1);
    std::unique_ptr<Vector> vr = *(v1.get()) + v2.get();
}

When using the overloaded operator (+) it looks a bit ugly but it's the way I found that worked when using unique_ptr<T>. One thing I'm doubting if I'm doing "correctly" is returning a unique_ptr<T>. Should I return a value instead and then create a unique_ptr<T> from that instead?

Answer 1

I dislike that your class mixes functionality (a mathematical vector) with memory management. A class should only do one of those. If I prefer shared_ptr I have to rewrite your whole Vector class.

There is actually a thing in C++ that is better than smart pointers, which is value semantics. People rarely screw up the lifetime management of an int, it just works naturally. When you do tricky lifetime management try to mimic the way int does it.

The operator + should be const (as well as GetX, GetY and GetZ). It should also work on Vectors and not only on pointers. The standard way to add Vectors is this:

Vector operator +(const Vector &lhs, const Vector &rhs){
    return Vector(lhs.GetX() + rhs.GetX(), lhs.GetY() + rhs.GetY(), lhs.GetZ() + rhs.GetZ());
}

Note that this is a free function, not a member function of Vector. Prefer free functions, because they work in more cases (like multiplying an int to a Vector) and are thus more consistent.

This allows you to do this:

Vector v1{ 1, 2, 3 };
Vector v2{ 6, 5, 4 };
auto v3 = v1 + v2;

Now, if you do want to support unique_ptr you can do so basically the same way:

std::unique_ptr<Vector> operator +(const std::unique_ptr<Vector> &lhs,
    const std::unique_ptr<Vector> &rhs){
    return std::make_unique<Vector>(*lhs + *rhs); //uses above operator
}

This allows you to do this:

auto v1 = std::make_unique<Vector>(1, 2, 3);
auto v2 = std::make_unique<Vector>(4, 5, 7);
auto v3 = v1 + v2;

All in all you seem to have a pretty good grip on operator overloading and smart pointers already.

Answer 2

No there is no need for smart pointers in the above code.

class Vector {
   // As Before 

    // Change this;   
    // Note: Pass by const reference.
    //       Also mark the method cost as you are not modifying this
    Vector operator + (Vector const& vector) const;

   // As before    
};

Then the add becomes:

Vector Vector::operator + (Vector const& vector) const {

    int resultX = x + vector.x;   // You be able to trust your own class getting values
    int resultY = y + vector.y;   // Getters and Setters are for other peoples code.
    int resultZ = z + vector.z;   // You are already tightly bound to your own code.

                                  // PS. Vector is automatically a friend of Vector.
                                  // So you can access the members

                                  // PPS Getters and setters are a horrible concept
                                  // and are considered bad as they cause tight coupling.

          // This may look like a copy but RVO will get the compiler to build this
          // in place at the destination.
    return Vector(resultX, resultY, resultZ);
}

Your code will now look more natural:

int main() {
    Vector v1(1, 2, 3);
    Vector v2(3, 2, 1);
    Vector vr = v1 + v2;
}