Am I using delete correctly here?

Question

I've just started combining my knowledge of C++ classes and dynamic arrays. I was given the advice that "any time I use the new operator" I should delete. I also know how destructors work, so I think this code is correct:

main.cpp

...
int main()
{
    PicLib *lib = new PicLib;
    beginStorage(lib);
    return 0;
}

void beginStorage(PicLib *lib)
{
...
    if (command != 'q')
    {
        //let's assume I add a whole bunch
            //of stuff to PicLib and have some fun here
        beginStorage(lib);
    }
    else
    {
        delete lib;
        lib = NULL;
        cout << "Ciao" << endl;
    }
}

PicLib.cpp

...

PicLib::PicLib()
{
    database = new Pic[MAX_DATABASE];
    num_pics = 0;
}

PicLib::~PicLib()
{
    delete[] database;
    database = NULL;
    num_pics = 0;
}
...

I fill my PicLib with a Pic class, containing more dynamic arrays. Pic's destructor deletes them in the same manner seen above. I think that delete [] database gets rid of all those classes properly.

So is the delete in main.cpp necessary? Everything looking hunky dory here?

Answer 1

There are a couple of problems:

int main() 
{ 
  PicLib *lib = new PicLib; 
  beginStorage(lib); 
  return 0; 
}

It is best to allocate and delete memory in the same scope so that it is easy to spot.

But in this case just declare it locally (and pass by reference):

int main() 
{ 
    PicLib  lib; 
    beginStorage(lib); 
    return 0; 
}

In beginStorage()

But I see no reason to manipulate a pointer. Pass it by reference and just use it locally.

void beginStorage(PicLib& lib)
{
 ....
}

In the PicLib class you have a RAW pointer: databases.

If you have a RAW pointer that you own (you create and destroy it) then you must override the compiler generated versions of the copy constructor and assignment operator. But in this case I see no reason touse a pointer it would be easier to just use a vector:

class PivLib
{
    private:
        std::vector<Pic>   databases;
};