Search code examples
c++memoryvirtual-destructor

Delete object inside a function: ugly, error-prone, inefficient, and usually not exception-safe


According to the following article: Why isn’t the destructor called at the end of scope?

Code that creates an object using new and then deletes it at the end of the same scope is ugly, error-prone, inefficient, and usually not exception-safe. For example:

 void very_bad_func()    // ugly, error-prone, and inefficient
    {
        X* p = new X;
        // use p
        delete p;  // not exception-safe
    }

My Code which I hope is not ugly:

I am creating an object of type TiXmlDocumentand the delete it by the end of the function.

void DataLoader::readXmlFile(const string & file)
{
    TiXmlDocument *doc = new TiXmlDocument(file.c_str());
    bool loadOkay = doc->LoadFile();
    TiXmlHandle hdl(doc);

    //work is done in here
    if(loadOkay)
    {
        TiXmlElement * pRoot = hdl.FirstChildElement().Element();//.FirstChildElement().Element();
        parseFile(pRoot);
    }
    else
    {
        cout <<"Error: "<< doc->ErrorDesc()<<endl;
    }
    //deallocate doc
    delete doc;
}

Question(s):

  • Should I use DataLoader::~DataLoader() {} destructor in order to insure that the object is deleted after I leave the scope of the function? without the need of explicitly deleting it delete doc.

As suggested I did the following:

TiXmlDocument doc(xmlFile.c_str());
bool loadOkay = doc.LoadFile();
TiXmlHandle hdl(&doc);

I am starting to think that using dynamic memory just like java and c# is not a good practice (should be used in a responsible way) in c++. If there is no real cause to use it then don't. If not handled correctly it will cause a memory leak which is hard to trace.



Solution

  • Either create your object without new:

    void func()
    {
        X p;
        // use p
    }
    

    Or if you must use new (for example, it's a big object) then use a smart pointer:

    void func()
    {
        std::unique_ptr<X> p(new X);
        // use p
    }
    

    Problem solved!