HELP PLEASE :(

[Spongboy98]

Okay so i am trying to make an expand function for a vector class that doubles the capacity (cap) and makes a temporary array( with a pointer). I'm supposed to delete the array with the old capacity in the process here's the description
"This function will double the capacity of the vector. This function should reallocate memory for the dynamically allocated array and update the value of capacity. Be careful to properly handle the case when capacity is 0 before calling expand(). Make sure you don't create a memory leak here."
what about my logic is wrong. I've tried to delete data but can't because i already have a destructor( not shown here) that does that. data is a private pointer variable which already has the capacity before it is expanded. I'm trying to get it to have double the capacity. The problem is i have to use this private expand function in all of my other public functions so i really can't start before I get this specific function correct.

TLDR: my question is why is it that i get segmentation fault (i know it has something to do with the data = new int line as its already been declared but why? code is below
void IntVector::expand() {
int * temp = new int[cap];
for (unsigned i = 0; i < sz; ++i) {
temp[i] = data[i];
}
if (cap == 0) {
cap = 1;
}
cap *= 2;
data = new int [cap];
for (unsigned i = 0; i < sz; ++i) {
data[i] = temp[i];
}

delete temp;
}

[Chervil]

    delete temp;

should be

    delete [] temp;

The function is unnecessarily allocating with new[] and copying all the data twice, which is inefficient.

Modify the value of cap at the start, then do the temp = new and the for loop.

Next use a temporary variable to swap the two pointers data and temp and last of all, delete [] temp.

(or instead of swapping them, first delete [] data, then set data = temp)

[Spongboy98]

Well this was the function that i found to cause seg fault after using expand (it was the for loop starting with for( i = sz - 2; i >= 0; i--) but i don't know why.
void IntVector::insert(unsigned index, int value) {
unsigned i;
at(index);
if (sz < cap) {
sz +=1;
for (i = sz -2; i >= 0; --i) {
if (i >= index) {
data[i+1] = data[i];
}
}
// data[index] = value;
cout << "array now: ";
for (i = 0; i < sz; ++i) {
cout << data[i] << endl;
}
}
else {
expand();
if (sz < cap) {
sz +=1;
cout << "!!!";
for (i = sz -2; i >= 0; i--) {
if (i >= index) {
data[i+1] = data[i];
}
}
// data[index] = value;
cout << "array now: ";
for (i = 0; i < sz; ++i) {
cout << data[i] << endl;
}
}
else {
cout << "ERROR: cap was not changed." << endl;
}
}
}

[Spongboy98]

Here's my new expand function. Unfortunately i get double free or corruption error when deleting data in here as i already have a destructor that does that(This is when i comment out the troublesome for loop i meant earlier). What would swapping two pointers look like?
void IntVector::expand() {
if (cap == 0) {
cap = 1;
}
cap = cap * 2;
int * temp = new int[cap];
// cap = cap * 2;
for (unsigned i = 0; i < sz; ++i) {
temp[i] = data[i];
}
delete data;

data = temp;

/*for (unsigned i = 0; i < sz; ++i) {
data[i] = temp[i];
} */

delete temp;
}

[Spongboy98]

Also i just saw your delete [] temp comments and applied them to the above code. Same issue.

[Chervil]

In the latest code, there is one new[] so you need one delete[]

void IntVector::expand() 
{
    if (cap == 0) {
        cap = 1;
    }
    cap = cap * 2;

    int * temp = new int[cap];
   
    for (unsigned i = 0; i < sz; ++i) {
        temp[i] = data[i];
    }
        
    delete [] data;  // [] to match new []

    data = temp;

} 

What would swapping two pointers look like?

You don't need that now, it was actually more complicated than necessary.

But it's the same as swapping any other type.
e.g.

    int x = 5;
    int y = 12;
 
    // now swap x and y
    int temp = x;
    x = y;
    y = temp;

The only thing that is different is the type is int * rather than int.

[Chervil]

The insert() function has unnecessary duplication of code.
You could just put

    if (sz >= cap)
        expand();

at the very start of that function, and then the rest can be done just once.

[Spongboy98]

Thanks for clearing that up but i still have a destructor (declared elsewhere) that already deletes data so i wouldn't be able to delete it again because of the double free error right? So swapping would be my only option right? In any case I've been at this program (this expand function to be exact, making it right so other functions can use it) for the past 4 hours, and my brain is starting to become fried. I really appreciate your help but I'm just going to "take the L" and get help on this in my office hours just so i have a better understanding.

[Chervil]

Thanks for clearing that up but i still have a destructor (declared elsewhere) that already deletes data so i wouldn't be able to delete it again because of the double free error right?

No.

The delete [] in the destructor is there to match the new[] in the constructor.

Think of it in terms of symmetry or balance. Every new has a corresponding delete. Sometimes they are close together, sometimes far apart. But so long as they pair off, it should make sense.