project.exe has triggered a breakpoint after system("pause")

Question

I've created my own vector, and when I try to close the console in my driver, I hit enter and get the breakpoint.

#include "MyVector.h"
#include <vector>

// Default constructor
MyVector::MyVector()
{
    theData = nullptr;
    vecSize = 0;
    vecCap = 0;
}

// Parameterized constructor
MyVector::MyVector(int vecCap)
{
    // Set vecSize to 0 and vecCap to user input (driver) plus 1 to account
    // for null terminator
    vecSize = 0;
    this->vecCap = vecCap;
    theData = new int[vecCap];
}

// Destructor
MyVector::~MyVector()
{
    // Run the clear() function
    clear();
}

// Copies the vector
void MyVector::copy(const MyVector& toCopy)
{
    // Set size and capacity to toCopy's
    this->vecCap = toCopy.vecCap;
    this->vecSize = toCopy.vecSize;

    // Create a temporary pointer array of ints
    int* tempArray = new int[];

    // Copy data from toCopy to new array
    for (int i = 0; i < toCopy.size(); i++)
    {
        tempArray[i] = toCopy.theData[i];
    }

    // Point theData to the tempArray
    theData = tempArray;
}

// Clears theData and resets vecCap and vecSize to 0
void MyVector::clear()
{
    // Check if theData is null
    if (theData != nullptr)
    {
        // Delete theData from heap and set to nullptr
        delete[] theData;
        theData = nullptr;

        // Set vecSize and vecCap to 0
        vecSize = 0;
        vecCap = 0;
    }
}

// Returns size of the vector
int MyVector::size() const
{
    return vecSize;
}

// Returns capacity of the vector
int MyVector::capacity() const
{
    return vecCap;
}

// Push input values into vector
void MyVector::push_back(int n)
{
    // Check if vecSize is too big for vecCap
    if (vecSize >= vecCap)
    {
        // Double vecCap through grow() function
        grow(vecCap);
    }

    // Set theData at element vecSize to user input n, increment vecSize
    theData[vecSize] = n;
    vecSize++;
}

// Returns index value of vector
int MyVector::at(int vecIdx) const
{
    // Check if vecIdx is within bounds
    if (vecIdx >= 0 && vecIdx <= vecSize)
    {
        // Return vector index
        return theData[vecIdx];
    }
    else
    {
        // Display out of bounds index
        throw vecIdx;
    }
}

// Doubles the size of the vector capacity
void MyVector::grow(int curCap)
{
    // Check if curCap is 0 ro less
    if (curCap <= 0)
    {
        // Set vecCap to CAP_GROWTH -1
        vecCap = CAP_GROWTH - 1;
    }
    else
    {
        // Increase curCap by CAP_GROWTH (doubled)
        vecCap = CAP_GROWTH * curCap;
    }

    // Create new array
    int* newArray = new int[vecCap];

    // Copy data to new array
    for (int idx = 0; idx < vecSize; idx++)
    {
        newArray[idx] = theData[idx];
    }

    // Delete theData
    delete[] theData;

    // Point theData to new array
    theData = newArray;
}

//
MyVector& MyVector::operator=(const MyVector& rho)
{
    // Check if the implicit object's address is the same as rho
    if (this != &rho)
    {
        // Clear the implicit object
        this->clear();
        // Copy the 
        this->copy(rho);
    }
    return *this;
}

// 
ostream& operator<<(ostream& out, const MyVector& rho)
{
    for (int idx = 0; idx < rho.size(); idx++)
    {
        // Output index of rho, separated by a space
        out << rho.at(idx) << " ";
    }
    return out;
}

I've checked for somewhere I may have tried to re-delete a pointer, but I can't find anything, any it doesn't say why the exception was thrown. Any tips?

Answer 1

Your class does not follow the rule of 3:

What is The Rule of Three?

You are lacking a copy constructor. Your copy function is not a copy constructor. You should implement a proper copy constructor first.

In addition, your copy function has a memory leak, since you never deleted the old data.

Here is an example of a copy constructor

#include <algorithm>
//...
MyVector::MyVector(const MyVector& toCopy) : vecCap(toCopy.vecCap),
                                             vecSize(toCopy.vecSize),
                                             theData(new int[toCopy.capacity()])
{
    std::copy(toCopy.theData, toCopy.theData + toCopy.size(), theData);       
}

Now, if you really want to keep the copy function (you really don't need it, but for argument's sake you want to keep it), you can use the above copy constructor:

// Copies the vector
void MyVector::copy(const MyVector& toCopy)
{
    MyVector temp(toCopy);  // calls the copy constructor
    std::swap(temp.vecCap, vecCap);
    std::swap(temp.vecSize, vecSize);
    std::swap(temp.theData, theData);
}

Note that all we did was create a temporary copy, and swap out the temporary copy's data with the current data. The temporary dies off when the function return, taking along with the old data.

Last, your assignment operator can be written using your copy function.

MyVector& MyVector::operator=(const MyVector& rho)
{
    copy(rho);
    return *this;
}

So in a sense, your copy function served a purpose, if the only purpose was to move some of the code around.

Actually, this is what you should have coded for your assignment operator, even without the changes I suggested. All the assignment operator should have been doing was call copy and return the current object.

Answer 2

In copy function, clear old data, create new data based on capacity:

void MyVector::copy(const MyVector& x)
{
    clear();
    vecCap = x.vecCap;
    vecSize = x.vecSize;
    theData = new int[vecCap];

    for (int i = 0; i < x.size(); i++)
        theData[i] = x.theData[i];
}

Also you are using the same names in argument, it's not wrong the way you are doing it, but it's easier to use a different name:

MyVector::MyVector(int c)
{
    vecSize = 0;
    vecCap = c;
    theData = new int[vecCap];
}