I tried to make my custom Vector class, with a template class.
I expect I can put my Vector<int>
into a Vector<Vector<int>>
variable. At least that was what I was hoping for... but it keeps crashing at the destructor code.
Here's my code.
#include <iostream>
#include <string>
template <typename T>
class Vector {
T* data;
int capacity;
int length;
public:
typedef T value_type;
Vector() {}
Vector(int n) : data(new T[n]), capacity(n), length(0) {}
void push_back(T input) {
data[length ] = input;
}
T operator[](int i) { return data[i]; }
virtual ~Vector() { if (data) delete[] data; }
};
int main() {
Vector<Vector<int>> v(3);
Vector<int> vv(4);
v.push_back(vv);
}
So I thought, maybe I should use a copy constructor, since it seems the problem is that v
is being deleted before vv
. Well, if I just comment out the destructor code, it will work, but that doesn't seem right to me...
So I made a custom copy constructor like this:
Vector(const T& other) {
}
But it give me an error, saying "ambiguous overloading"... looking back, of course it is wrong, since T
of data
is different from T
of other
...
How can I make my custom Vector
class work? (i.e. I want push_back work as I intended...)
CodePudding user response:
The general issue
In class design, especially when memory/resource allocation is involved, you typically need to follow the "Rule of Five" (which used to be the "Rule of Three" before C 11):
If you implement any of:
- A destructor
- An copy/move assignment operator
- A copy/move constructor
then you will probably need to implement all of them.
The reason is that each of them likely needs to have some resource management logic, beyond what the language gives you as a default.
The signatures of these five methods, for your class, would be:
Method | Signature |
---|---|
Copy constructor | Vector(const Vector&) |
Move constructor | Vector(Vector&&) |
Copy assignment operator | Vector& operator=(const Vector&) |
Move assignment operator | Vector& operator=(Vector&&) |
Destructor | ~Vector() or virtual ~Vector() |
Your specific class
In your specific case, there are several concrete problems:
- As @UnholySheep suggests, you mis-declared the copy constructor.
- You implemented a default constructor; but - it doesn't allocate anything, nor does it initialize anything! The
data
pointer holds arbitrary junk, and when you try to free it, bad things are likely to happen. - You are performing quite a lot of copying of
T
values, which for the outer vector would beVector<int>
values - and that can get expensive. - Even if you fixed the above, you should still implement the missing methods from the "rule of five".
CodePudding user response:
Your default constructor leaves the object completely uninitialized.
Consider what happens when you declare a
Vector<int> foo;
foo
essentially gets a random memory address as data
, a random length
and capacity
. This will give fireworks if you free it.
Perhaps you sidestepped this issue by always creating your vector with a predefined size. Luckily, trying to create/destroy a Vector<Vector<int>>
brings this to light, because the Vector<int>[]
inside your container still contains these ticking timebombs.