CODEWITHSUNDEEP
c++pointersvectorresizebad-alloc
Lijie Zhou
Lijie Zhou

Reputation: 75

bad_alloc error when implementing vector resize function

I am trying to implement Vector::resize() function in C++. I think I handled each situation but still get a bad_alloc error. Three cases in this resize implementation:

  1. when new_size is smaller than the old_size (in my code, size);
  2. when new_size is bigger than size but smaller than capacity;
  3. when new_size is bigger than capacity

Here is my code:

void Vector::resize(int new_size)
{
    //if the new_size is smaller, make the size smaller, don't need to worry about memory
    //the content is reduced to its first n elements
    //remove those beyond and destroy them
    
    if(new_size < size){
        
        for(int i = size-1; i > new_size; i--){
            erase(i);
        }
        size = new_size;
    }
    
    //if the new_size is bigger
    //case 1: new_size is smaller than capacity
    //inserting at the end of as many elements as needed
    if(new_size > size && new_size < capacity){
        
        for(int i=size; i < new_size; i++){
            insert(i, 0.0);
        }
        size = new_size;
        
    }
    
    //case 2: new_size is greater than capacity
    //increase the capacity of the container
    
    //increase the capacity to new_size
    double *tmp_data = new double(new_size);
    
    //transfer data to tmp_data
    for(int i=0; i < size; i++)
    {
        tmp_data[i] = data[i];
    }
    
    data = tmp_data;
    delete [] data;
    
    size = new_size;
    capacity = new_size; 
}

Upvotes: 0

Views: 979

Answers (2)

Alexis Wilke
Alexis Wilke

Reputation: 20771

As pointed out by Barry, you had a few bugs in the original.

Here is your code with a few suggestions:

void Vector::resize(int new_size)
{
    if(new_size <= size) {     // use "<=" here

        // if you are dealing with doubles, what is there to erase?
        // (I would remove that loop)
        for(int i = size-1; i > new_size; i--){
            erase(i);
        }
        size = new_size;

        return;   // you're done here, you can return
    }
    
    if(new_size <= capacity) {    // again "<=", if you return, then no need for anything more

        // "insert()" sounds confusing, you're doing a set() here
        for(int i=size; i < new_size; i++){
            insert(i, 0.0);
        }
        size = new_size;

        return;   // again, you can now return, you're done
    }
    
    double *tmp_data = new double(new_size);
    
    // you could use std::copy() here
    for(int i=0; i < size; i++)
    {
        tmp_data[i] = data[i];
    }
    
    // as indicated by Barry, you could inverse these
    delete [] data;
    data = tmp_data;
    
    size = new_size;
    capacity = new_size; 
}

I removed all your comments and put my own for you perusal.

Another way to handle 3 cases like these is to use the else keyword like so:

if(new_size <= size) {
    ...
} else if(new_size <= capacity) {
    ...
} else {
    ...
}

And to make it clear, you could have three sub-functions you call from each ... cases. That way you see the main logic of your resize() function.

Upvotes: 0

Barry
Barry

Reputation: 303347

There are several things wrong with this code. The one that jumps out first is:

//increase the capacity to new_size
double *tmp_data = new double(new_size);

You intend to allocate an array, but are actually allocating a single double. You meant:

double *tmp_data = new double[new_size];

Although, once you fix that...

data = tmp_data;
delete [] data;

You want to do those in the opposite order, otherwise you're leaving yourself with a deleted member.

And once you fix that, you want to return from your cases early. You have three cases (not two, as your comments would suggest), and you only want to reallocate in the case that you actually need to (i.e. case #3). As-is, you're reallocating in all your cases.

Upvotes: 5

Related Questions