Friday, March 17, 2017

Make path-to-string utility as static function

I have a string utility that converts the boost::filesystem::path into a string. The current implementation was done in the following way.
/** header file **/

using convert_typeX = std::codecvt_utf8<wchar_t>;

class Path {

   ...
   ...

private:
   wstring_convert<convert_typeX, wchar_t> converterX;

};


/** implementation file **/

Path::Path(path thePath)
    : rootPath(thePath), searchDone(false)
{
    string filename = converterX.to_bytes(thePath.wstring());
    string rootPath = converterX.to_bytes(this->rootPath.wstring());

    ...
}
This utility is spread across in every class that require path-to-string conversion. After some time when I look back on this code I felt that the design approach is very unpractical and quite troublesome too. Since this utility is used in everywhere, I just wonder, would it be better if I make it static. I'm just trying to simplify things, complex thing is tiring me.
/** header file **/

using convert_typeX = std::codecvt_utf8<wchar_t>;

class FileHelper {

   ...
   ...

public:
   static string convertPathToString(boost::filesystem::path thePath) {
      wstring_convert<convert_typeX, wchar_t> converterX;
      return converterX.to_bytes(thePath.wstring());
   }


private:
   wstring_convert<convert_typeX, wchar_t> converterX;

};


/** implementation file **/

Path::Path(path thePath)
    : rootPath(thePath), searchDone(false)
{
    string filename = FileHelper::convertPathToString(thePath);
    string rootPath = FileHelper::convertPathToString(this->rootPath);

    ...
}
With the new approach, I centralize that piece into FileHelper (a utility class specializes in file). When Path objects need it, just direct call on convertPathToString() from the class.

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();
}