Search code examples
c++exceptionfile-iodestructorstdio

Is it safe to call remove() to delete files in destructor?


I have a class that creates a few temp files when certain member functions are called. I want those files to be removed whenever the class goes out of scope (normally or due to exception), so I'd like to remove them in the destructor:

#include <cstdio>
#include <string>
class MyClass
{
    //implementation details

    //Names of temp files
    std::string tempFile1, tempFile2,tempFile3;

    ~MyClass()
    {
         remove(tempFile1.c_str());
         remove(tempFile2.c_str());
         remove(tempFile3.c_str());
    }
};

The problem is that if the destructor is called due to an exception, then it is probable that not all the 3 temp files have been created. According to cpluscplus.com, in this case the remove() function will return a non-zero value and write something to stderr. But since it's a C function, there won't be an exception.

I know that destructors should not throw. What about errors like this? Is it recommended to write a destructor like this one?


Solution

  • What you have shown will work fine. But I generally prefer a more RAII approach, eg:

    #include <cstdio>
    #include <string>
    
    struct RemovableFile
    {
        std::string fileName;
        bool canRemove;
    
        RemovableFile() : canRemove(false) {}
        ~RemovableFile(){ if (canRemove) remove(fileName.c_str()); }
    };
    
    class MyClass
    {
        ...
        //Names of temp files
        RemovableFile tempFile1, tempFile2, tempFile3;
        ...
    };
    
    void MyClass::doSomething()
    {
        ...
        tempFile1.fileName = ...;
        ...
        if (file was created)
            tempFile1.canRemove = true;
        ...
    };
    

    Or maybe something more like this:

    #include <cstdio>
    #include <string>
    #include <fstream>
    
    struct RemovableFile
    {
        std::string  fileName;
        std::fstream file;
    
        ~RemovableFile() { if (file.is_open()) { file.close(); remove(fileName.c_str()); } }
    
        void createFile(const std::string &aFileName)
        {
            file.open(aFileName.c_str(), ...);
            fileName = aFileName;
        }
    };
    
    class MyClass
    {
        ...
        //Names of temp files
        RemovableFile tempFile1, tempFile2, tempFile3;
        ...
    };
    
    void MyClass::doSomething()
    {
        ...
        tempFile1.createFile(...);
        ...
    };