Abdulrahman Mansour
Abdulrahman Mansour

Reputation: 11

Error while trying to declare a push back method in a vector class that i created

I created a class that mimics the action of C++ vector by creating a dynamic array, I try to create a push back method for that class which first checks if the array is filled, if so it will: 1- Copy the contents of current array to a temporary array with the double size 2- Delete the old dynamic array 3- Create a new dynamic array with the double size of the old array (same size of temporary array) 4- Copy contents of the temporary array to the new dynamic array

the error is while I use the following code, I can only double the size of array once, then it throws to error: HEAP[ConsoleApplication1.exe]: Invalid address specified to RtlValidateHeap( 016C0000, 016CDB98 ) ConsoleApplication1.exe has triggered a breakpoint.

#include <iostream>

using namespace std;

class SimpleVector {
private:
    int* item; //pointer to the dynamic array (the vector)
    int size;
    int numElements;

public:
    SimpleVector(int size) {
        this->size = size;
        this->numElements = 0;
        this->item = new int[this->size];
    }
    SimpleVector():SimpleVector(10){}

    void pushBack(int element) {
        //check for overflow
        if (numElements >= size) {
            int newSize = size * 2;
            
            int* temp = new int[newSize]; // temporary array with the double size to hold old array elements
            for (int i = 0; i < numElements; i++) {
                temp[i] = item[i];
            }
            delete[] item;
            size = newSize;


            //****ERROR IS IN THIS PART****
            int* item = new int[size]; 
            for (int i = 0; i < numElements; i++) {
                item[i] = temp[i];
            }
            //****END OF THE PART CONTAINING ERROR****


            item[numElements++] = element;
            cout << "Added: " << element << endl;
            cout << "Size is: " << size << endl;
        }
        else {
        item[numElements++] = element;
        cout << "Added: " << element << endl;
        cout << "Size is: " << size << endl;
    }
    }
};

int main() {
    
    SimpleVector v1(2);
    v1.pushBack(1);
    v1.pushBack(2);
    v1.pushBack(3);
    v1.pushBack(4);
    v1.pushBack(5);
    v1.pushBack(6);
    v1.pushBack(7);
    

    return 0;
}

This program pushs the first 4 items and then throws into error when trying to double the size form to 8

when I just replace that part containing error with:

item = temp

it works fine, but I can't understand why this happens.

Upvotes: 0

Views: 99

Answers (1)

WhozCraig
WhozCraig

Reputation: 66194

Your code has several problems. I suggest you read about how to properly comply with Rule of Three/Five/Zero , which I'm not going to cover here. I only imagine this is for some nefarious academic purpose, so I will try to be brief, but please read the linked article.

That said, this:

int *item = new int[size];
for (int i = 0; i < numElements; i++)
{
    item[i] = temp[i];
}

is pointless. This starts a cascade of bad actions that only gets worse as it rolls along.

  • You already have a member variable item. This code declares a local variable named item whose name hides the member item. Therefore the initialized value from the new operator is stored in the local, not the member.
  • That memory now pointed to by the local var item (not the member) is lost on exit of the function, since nothing but the local item ever points to it.
  • That additional allocation is pointless to begin with. You already made a new vector copy, pointed to by temp, and already copied all the legitimate items from memver-var item memory to temp memory. The code as-written leaks that temp allocation as well.
  • You destroy the member-var item memory, leaving the pointer now dangling, and any usage thereafter by dereference or eval invokes undefined behavior.

So, in summary, you make a valuable extended copy, leak it, make a worthless copy, leak it too, and end up leaving with a member variable that is no longer pointing at anything defined.

That entire function could look more like this:

void pushBack(int element)
{
    // check for overflow
    if (numElements >= size)
    {
        int newSize = size * 2;

        int *temp = new int[newSize];
        for (int i = 0; i < numElements; i++)
        {
            temp[i] = item[i];
        }
        delete[] item;
        size = newSize;
        item = temp;
    }
    item[numElements++] = element;
    cout << "Added: " << element << endl;
    cout << "Size is: " << size << endl;
}

Upvotes: 2

Related Questions