Thursday, March 2, 2017

Constructor pattern or code design error?

There is a minor defect happened in my program where the program only able to capture the files in the last recursive loop. Below is the code snippet of the code:
void FilePath::captureFiles(path searchPath) {

            ...
            ...

            // it is a directory
            if( is_directory(status(*it)) ) {
                BOOST_LOG_TRIVIAL(info) << "The file is a directory";

                captureFiles(*it);
            }
            // it is a file
            else {
                ...
                ...

                if( bPlus ) {
                    bPlus = false;
                    indexFile << "+" << filename << endl;

                    BOOST_LOG_TRIVIAL(info) << "FilePath::captureFiles : The file is capture into index file with '+' sign";
                }
                else if( !bMinus ) {
                    bMinus = false;
                    indexFile << filename << endl;

                    BOOST_LOG_TRIVIAL(info) << "FilePath::captureFiles : The file is capture into index file";
                }
            }

    indexFile.close();

    ...
    ...
}
The root cause happened when there is no more directory were found in the path and the program will flow into the else section. Once the file name was captured into indexFile, the indexFile were close!

Then where does this variable get initialized?
FilePath::FilePath(path thePath)
    : rootPath(thePath), searchDone(false)
{
    ...
    ...

    // capture the index file content into memory
    indexFilePath = rootPath + string(1, path::preferred_separator) + filename + ".i";

    if( exists(indexFilePath) ) {
        captureExistingFile();
        bFileExist = true;
    }

    indexFile.open(indexFilePath, ios_base::in | ios_base::out | ios_base::app);
}
Notice that the indexFile only got initialize once in the constructor, and its memory was free in the last loop of the recursive function, I have no way to reopen it inside the recursive loop.

A good programming practice for this case is that close it in the destructor since it got initialize in the constructor, then the problem will fix automatically.
FilePath::~FilePath()
{
    ...

    if( indexFile.is_open() )
        indexFile.close();
}

No comments: