Should I write the end of the file in the destructor?

Question

I have some code which looks a bit like this:

void writeToStream( std::ostream & outputStream )
{
    MyXmlWriter xmlWriter{ outputStream };
    xmlWriter.addNode();
    xmlWriter.addNode();
    xmlWriter.close(); // should this be called in `MyXmlWriter` destructor?
}

The close function writes some xml close tags so the file can be parsed properly. The constructor writes the header of the xml file. One could consider xmlWriter.close(); clean-up code. It is common advice for C++ to put clean-up code into destructors. This way you can never forget to clean-up properly. However, in our case the clean-up code could throw. (Imagine that file could have exceptions enabled, writes to a file can fail.) Therefore, if the close() function is called in the destructor, then it should be wrapped in a try-catch block which eats all exceptions thrown:

MyXmlWriter::~MyXmlWriter() 
{
    try
    {
        close();
    }
    catch (...)
    {
    }
}

However, in this case the caller will not be notified about any errors. The function writeToStream() could fail to write the closing xml tags to the file without the caller knowing about it. What is best practice in this situation?

Answer 1

What are you closing? In general, closing a file open for writing (std::ostream, FILE* or a system dependent file descriptor) must be done before destruction, so that you can check for errors after having done the close, and report them. There are exceptions, however, and in particular, classes which wrap an open file should generally close it in their destructor (without checking for errors, since there's nothing you can do about them), in order to ensure proper clean up in case of an exception.

Presumably, an exception before the close means that there has been an error already, and the file being written will not be used. I generally wrap output in a class with a commit function. commit closes, and checks for errors. If the destructor is called before commit, it closes, without checking for errors, and then removes the file being written, since it is probably not complete or usable.

Answer 2

Swallowing exceptions is usually a "worst practice" because it defeats the purpose of throwing in the first place.

But in this case, you really only want a subset of functionality in the destructor, not including flushing which is a "bonus" but incurs the potential to throw. There may still be side effects to trying to flush at all, such as waiting unnecessarily for a network timeout where one already occurred.

As James Kanze mentioned, best practice is to manually flush before the destructor runs, which precludes the exceptional condition in the destructor.

In the future C++ might support transactions better. But for now, your approach is reasonable. In any case, it is how the destructor of std::filebuf is specified to work:

Effects: Destroys an object of class basic_filebuf<charT,traits>. Calls close(). If an exception occurs during the destruction of the object, including the call to close(), the exception is caught but not rethrown (see 17.6.5.12).