0

I am trying to shrink an array of *bool but I am not sure if it is being deleted correctly.

This is my source code...

bool *oldStore;


void shrinkArray(int i)
{
    int k;
    bool *newStore;
    for(k=0; k<i; k++)
    {
        newStore[k] = oldStore[k];
    }

    for(; k<originalSize; k++)
    {
        delete[] oldStore[k];
    }

    delete[] oldStore;
    oldStore = newStore;
}

For example, if I wanted to shrink the array to 5 and the original size of the array was 15, it would keep the first five and delete the last ten, however I am not too sure if my memory is being managed correctly.

Stu User
  • 77
  • 2
  • 3
  • 8
  • 3
    You should not delete each entry in the array. Only `delete` what you `new`, and only `delete[]` what you `new[]`. However in your case there's no need for dynamically allocated arrays, when you think "dynamic array" you should instead think [`std::vector`](http://en.cppreference.com/w/cpp/container/vector). – Some programmer dude Dec 12 '14 at 08:48
  • You never even initialise the pointer `newStore`, or allocate anything. This will probably crash on the first use of `newStore[k]`. But apart from being incorrect on many counts, the approach is horribly primitive anyway. – Marc van Leeuwen Dec 12 '14 at 09:30

3 Answers3

1

Simplest code would be:-

void shrinkArray(int i)
{
    int k;
    bool *newStore = new bool[i];
    for(k=0; k<i; k++)
    {
        newStore[k] = oldStore[k];
    }

    delete [] oldStore;   //assuming oldstore was allocated using new []..
    oldStore = newStore;

}

ravi
  • 10,994
  • 1
  • 18
  • 36
1

Think about you method design before going into the coding specifics. I assume you have an ínt array that you new somewhere in your code.

  1. Think about who "owns" this array? It's probably no good idea to create the int array at some place and to simply delete[] it somewhere else. Check out the following links: What is a smart pointer and when should I use one?

  2. Think about what should happen to your newStore array. Is it supposed to replace the oldStore or do you want both arrays to exist in parallel. If you simply put the newStore on the heap who/when and where are you going to delete[] it again.

Community
  • 1
  • 1
markus
  • 1,631
  • 2
  • 17
  • 31
0

Your code is wrong. You declared pointer newStore but neither initialize it nor allocated memory that would be pointed to by this pointer

bool *newStore;

So the next loop has undefined behaviour.

for(k=0; k<i; k++)
{
    newStore[k] = oldStore[k];
}

Moreover if each element of the array pointed to by pointer oldStore has type bool * that is in turn is a pointer then oldStore itself shall have type bool **

If so then the correct function could look like

void shrinkArray( int n )
{
    if ( n < originalSize )
    {
        bool **newStore  = new bool * [n];

        int i = 0;
        for ( ; i < n; i++ ) newStore[i] = oldStore[i];

        for ( ; i < originalSize; i++ ) delete oldStore[i];

        delete [] oldStore;

        oldStore = newStore;
        originalSize = n;
    }
}

Take into account that oldStore also shall have type bool **.

Otherwise if each element of the original array has type bool then the code will look like

void shrinkArray( int n )
{
    if ( n < originalSize )
    {
        bool *newStore  = new bool [n];

        int i = 0;
        for ( ; i < n; i++ ) newStore[i] = oldStore[i];
        // Or
        // std::copy( oldStore, oldStore + n, newStore );

        delete [] oldStore;

        oldStore = newStore;
        originalSize = n;
    }
}

Take into account that it would be much better and simpler to use standard container std::vector<bool *> or std::vector<bool> depending on the type of the element of the container.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335