Reputation: 51
I am doing a project for school and it requires a copy constructor, destructor, etc. When I use the copy constructor an error is thrown saying the array is empty, I assume the copy constructor is not working.
When I delete the copy constructor the program works, meaning the issue is probably happening within that function.
template<class T>
class DynArray {
public:
DynArray<T>(){
ptr = new T[Capacity];
this->Capacity = 2;
this->Size = 0;
}
DynArray<T>(T n) {
ptr = new T[Capacity];
this->Capacity = n;
this->Size = 0;
}
DynArray<T>(const DynArray& orig) {
cout << "Copy" << endl;
ptr = new T[Size];
*ptr = *(orig.ptr);
}
DynArray<T>& operator=(const DynArray<T>& orig) {
if(this != &orig) {
delete[] ptr;
ptr = new T[Size];
*ptr = *(orig.ptr);
}
return *this;
}
void push_back(const T& n) {
if (Size >= Capacity) {
adjust(Capacity * 2);
}
ptr[Size] = n;
Size++;
}
void adjust(T a) {
cout << "grow" << endl;
T* arr = new T[a];
for (int i = 0; i < Capacity; ++i) {
arr[i] = ptr[i];
}
Capacity = a;
ptr = arr;
}
T& back() {
if(Size == 0) {
throw runtime_error("Array is empty");
}
return ptr[Size - 1];
}
T& front() {
if(Size == 0) {
throw runtime_error("Array is empty");
}
return ptr[0];
}
private:
T* ptr = nullptr;
int Capacity;
int Size;
main:
#include <iostream>
#include "dynarray.h"
using namespace std;
int main( )
{
const char START = 'A';
const int MAX = 12;
// create a vector of chars
DynArray<char> vectD;
// push some values into the vector
for (int i = 0; i < MAX; i++)
{
vectD.push_back(START + i);
}
// remove the last element
vectD.pop_back();
// add another value
vectD.push_back('Z');
// test memory management
DynArray<char> vectD2 = vectD;
// display the contents
cout << "\n[";
for (int i = 0; i < vectD2.size() - 1; i++)
{
cout << vectD2.at(i) << ", ";
}
cout << "..., " << vectD2.back() << "]\n";
DynArray<char> vectD3;
vectD3 = vectD2;
cout << "\n[";
for (int i = 0; i < vectD3.size() - 1; i++)
{
cout << vectD3.at(i) << ", ";
}
cout << "..., " << vectD3.back() << "]\n";
vectD3.front() = '{';
vectD3.back() = '}';
cout << vectD3.front();
for (int i = 1; i < vectD3.size() - 2; i++)
{
cout << vectD3.at(i) << ", ";
}
cout << vectD3.at(vectD3.size()-2) << vectD3.back() << endl;
}
Later in my code I have it set to throw a runtime_error if Size == 0, it does throw the error meaning that the array is empty. Is the copy constructor copying correctly? Main cannot be changed, it is a given from the prof.
UPDATE: I changed the copy constructor to copy all of the elements of the array, but the runtime_error is still returning saying the array is empty.
template<class T>
class DynArray {
public:
DynArray<T>(){
ptr = new T[Capacity];
this->Capacity = 2;
this->Size = 0;
}
DynArray<T>(T n) {
ptr = new T[Capacity];
this->Capacity = n;
this->Size = 0;
}
DynArray<T>(const DynArray& orig) {
cout << "Copy" << endl;
ptr = new T[Size];
for (int i = 0; i < Size; i++) {
ptr[i] = orig.ptr[i];
}
}
DynArray<T>& operator=(const DynArray<T>& orig) {
if(this != &orig) {
delete[] ptr;
ptr = new T[Size];
for (int i = 0; i < Size; i++) {
ptr[i] = orig.ptr[i];
}
}
return *this;
}
void push_back(const T& n) {
if (Size >= Capacity) {
adjust(Capacity * 2);
}
ptr[Size] = n;
Size++;
}
void adjust(T a) {
cout << "grow" << endl;
T* arr = new T[a];
for (int i = 0; i < Capacity; ++i) {
arr[i] = ptr[i];
}
Capacity = a;
ptr = arr;
}
T& back() {
if(Size == 0) {
throw runtime_error("Array is empty");
}
return ptr[Size - 1];
}
T& front() {
if(Size == 0) {
throw runtime_error("Array is empty");
}
return ptr[0];
}
private:
T* ptr = nullptr;
int Capacity;
int Size;
Upvotes: 1
Views: 347
Reputation: 9062
Your copy constructor,
DynArray<T>(const DynArray& orig) {
cout << "Copy" << endl;
ptr = new T[Size];
*ptr = *(orig.ptr);
}
does not perform a deep copy.
A pointer, such as your ptr
, is just an index to a resource (some memory block). If you copy it, the memory block is still the same and unique, you just have copied its address.
In order to perform a deep copy, you need to allocate a new block of memory (which you do), and then copy it. To copy a block of memory means to copy each of its elements. Here, you copy only the first. Therefore,
DynArray<T>(const DynArray& orig) {
cout << "Copy" << endl;
ptr = new T[Size];
for (size_t i = 0; i < Size, i++)
ptr[i] = orig.ptr[i];
}
Note that a[i]
is the same as *(a+i)
, which means "the thing at the i'th position in the memory block".
Even better, you can use std::copy
to hide the details of the deep copy operation, thus replacing the for loop:
std::copy(std::begin(orig.ptr), std::end(orig.ptr), std::begin(ptr));
in C++11, or:
std::copy(orig.ptr, orig.ptr + Size, ptr);
Upvotes: 0
Reputation: 596332
There are quite a few mistakes in your class.
The default constructor uses Capacity
to allocate the array, before Capacity
has been initialized.
Your reserving constructor is declared wrong. Its input parameter should be an int
instead of a T
. And it suffers from the same Capacity
error as your default constructor.
Your copy constructor suffers from the same initialization error as well, only with Size
instead. And it does not perform a deep copy at all. It is merely copying the ptr
pointer from one class instance to another without regard to what is being pointed to. Same with your copy assignment operator.
Your adjust()
method is also declared wrong, and it is leaking memory.
With that said, try something more like this instead:
#include <algorithm>
#include <utility>
template<class T>
class DynArray
{
public:
DynArray(int n = 2)
: ptr(new T[n]), Capacity(n), Size(0)
{
}
DynArray(const DynArray& orig)
: DynArray(orig.Size)
{
std::cout << "Copy" << std::endl;
std::copy(orig.ptr, orig.ptr + orig.Size, ptr);
Size = orig.Size;
}
DynArray(DynArray&& orig)
: ptr(nullptr), Size(0), Capacity(0)
{
orig.swap(*this);
}
~DynArray()
{
delete[] ptr;
}
DynArray& operator=(const DynArray& orig)
{
if (this != &orig) {
DynArray(orig).swap(*this);
}
return *this;
}
DynArray& operator=(DynArray&& orig)
{
DynArray(std::move(orig)).swap(*this);
return *this;
}
void swap(DynArray &other)
{
std::swap(other.ptr, ptr);
std::swap(other.Capacity, Capacity);
std::swap(other.Size, Size);
}
void push_back(const T& n)
{
if (Size >= Capacity) {
grow();
}
ptr[Size] = n;
++Size;
}
void pop_back()
{
if (Size <= 0) {
throw std::runtime_error("Array is empty");
}
ptr[Size - 1] = T();
--Size;
}
T& front()
{
if (Size <= 0) {
throw std::runtime_error("Array is empty");
}
return ptr[0];
}
T& back()
{
if (Size <= 0) {
throw std::runtime_error("Array is empty");
}
return ptr[Size - 1];
}
T& at(int i)
{
if ((i < 0) || (i >= Size)) {
throw std::out_of_range("Index out of range");
}
return ptr[i];
}
T& operator[](int i)
{
return ptr[i];
}
int size() const {
return Size;
}
int capacity() const {
return Capacity;
}
private:
T* ptr = nullptr;
int Capacity = 0;
int Size = 0;
void grow()
{
std::cout << "grow" << std::endl;
DynArray newArr(Capacity * 2);
std::copy(ptr, ptr + Size, newArr.ptr);
newArr.Size = Size;
newArr.swap(*this);
}
};
Then, you should consider throwing all of this away and just use std::vector
instead, which handles all of these details for you:
#include <vector>
template<class T>
class DynArray
{
public:
DynArray(int n = 2)
{
vec.reserve(n);
}
void push_back(const T& n)
{
vec.push_back(n);
}
void pop_back()
{
vec.pop_back();
}
T& front()
{
return vec.front();
}
T& back()
{
return vec.back();
}
T& at(int i)
{
return vec.at(i);
}
T& operator[](int i)
{
return vec[i];
}
int size() const {
return vec.size();
}
int capacity() const {
return vec.capacity();
}
private:
std::vector<T> vec;
};
Upvotes: 1