patterncppMinor
C++ smart pointers and classes
Viewed 0 times
andsmartpointersclasses
Problem
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
Vector.cpp
Usage
When using the overloaded operator (+) it looks a bit ugly but it's the way I found that worked when using
Vector.h
#ifndef VECTOR
#define VECTOR
#include
class Vector {
private:
int x;
int y;
int z;
public:
Vector(int x, int y, int z);
~Vector();
std::unique_ptr 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; }
};
#endifVector.cpp
#include
#include
#include "Vector.h"
Vector::Vector(int x, int y, int z) : x(x), y(y), z(z) {
std::cout Vector::operator + (Vector* vector) {
std::unique_ptr result = std::make_unique(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
#include
#include "Vector.h"
int main() {
std::unique_ptr v1 = std::make_unique(1, 2, 3);
std::unique_ptr v2 = std::make_unique(3, 2, 1);
std::unique_ptr 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. One thing I'm doubting if I'm doing "correctly" is returning a unique_ptr. Should I return a value instead and then create a unique_ptr from that instead?Solution
I dislike that your class mixes functionality (a mathematical vector) with memory management. A class should only do one of those. If I prefer
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
The
Note that this is a free function, not a member function of
This allows you to do this:
Now, if you do want to support
This allows you to do this:
All in all you seem to have a pretty good grip on operator overloading and smart pointers already.
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 operator +(const std::unique_ptr &lhs,
const std::unique_ptr &rhs){
return std::make_unique(*lhs + *rhs); //uses above operator
}This allows you to do this:
auto v1 = std::make_unique(1, 2, 3);
auto v2 = std::make_unique(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.
Code Snippets
Vector operator +(const Vector &lhs, const Vector &rhs){
return Vector(lhs.GetX() + rhs.GetX(), lhs.GetY() + rhs.GetY(), lhs.GetZ() + rhs.GetZ());
}Vector v1{ 1, 2, 3 };
Vector v2{ 6, 5, 4 };
auto v3 = v1 + v2;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
}auto v1 = std::make_unique<Vector>(1, 2, 3);
auto v2 = std::make_unique<Vector>(4, 5, 7);
auto v3 = v1 + v2;Context
StackExchange Code Review Q#78502, answer score: 5
Revisions (0)
No revisions yet.