Monday, August 27, 2018

constructChildPath() need to be clean

Currently the constructChildPath() is taking 2 arguments, the first argument is a pointer to a FileNode type and the second argument is a string.
void FileBot::constructChildPath(FileNode *currentPath, string inputPath)
{
   ...
   ...
}
The purpose of having these 2 arguments is to allow the method to identify where should I scan the path and where should I start parking those paths in the list. I have been thinking for so long to revamp this method so that it doesn't take any argument for its process. I'm just trying to make it as clean as possible.

Here is my thought on the finding. In this method, the string argument, is the crucial part of this method because it tells me where should I start scanning the path. If it doesn't contain any value, then nothing would be scanned, the return list would be empty. A screenshot of the piece is as follows:
void FileBot::constructChildPath(FileNode *currentPath, string inputPath)
{
    vector<path> pathList;

    // do not proceed for root path
    if( inputPath == "" )
        return;

    // capture the paths/files list under the source
    copy(filesystem::directory_iterator(inputPath), filesystem::directory_iterator(), back_inserter(pathList));

    std::sort(pathList.begin(), pathList.end());

    // scan through path and those files sit in that path
    // scan for files in sub directory if there is any sub directory in that path
    for(vector<path>::const_iterator it(pathList.begin()); it != pathList.end(); ++it) {
        string file = FileHelper::convertPathToString((*it));

        if( is_directory(file) ) {
            FileNode *node = new FileNode();
            node->setName(file.substr(file.find_last_of(boost::filesystem::path::preferred_separator) + 1, file.length()));
            node->setType('d');
            node->setParentNode(currentPath);

            currentPath->sibling.push_back(*node);

            BOOST_LOG_TRIVIAL(info) << "subFolderName : " << node->getName().string() << " : " << node;

            constructChildPath(node, inputPath + string(1, boost::filesystem::path::preferred_separator) + node->getName().string());
        }

    ...
    ...
}
As mention in the code above, this is a recursive function, it will recursively dive into itself searching for more files when it sees a directory. Hence, the second argument of this method, inputPath, is constantly extending the directory underneath the current path. This method is only accessible through following method and it is highly dependent on the constructParentPath() because it needs a very important element for child path construction:
FileNode* FileBot::loadLocalFS(boost::filesystem::path inputPath)
{
    if( valiateRootPath(inputPath.generic_string()) == false ) {
        BOOST_LOG_TRIVIAL(info) << "Failed to validate input path";
        return nullptr;
    }

    constructParentPath(inputPath);
    constructChildPath(searchPath, inputPath.string());

    return searchPath;
}
Notice there is a junk variable called searchPath. I called it junk because it serves for temporary purpose. This variable was used to store the home path, which was generated in constructParentPath() as mention above, and then this variable will pass down to the constructChildPath(). This is what I mention the code is highly depends on others. Duhhh~

I'm restudy the code to see what I could improve about these junk code. As I first start, I remove the inputPath argument because I felt it was a redundant. It can generate on the spot from the currentPath, below is the upgrade version of this change:
void FileBot::constructChildPath(FileNode *currentPath)
{
    vector<path> pathList;

    // for the first time, retrieve the root path
    // bail otherwise
    if( currentPath == nullptr ) {
        return;
    }

    // construct path address
    string inputPath = constructPathAddress(currentPath);

    // capture the paths/files list under the source
    copy(filesystem::directory_iterator(inputPath), filesystem::directory_iterator(), back_inserter(pathList));

    std::sort(pathList.begin(), pathList.end());

    // scan through path and those files sit in that path
    // scan for files in sub directory if there is any sub directory in that path
    for(vector<path>::const_iterator it(pathList.begin()); it != pathList.end(); ++it) {
        string file = FileHelper::convertPathToString((*it));

        if( is_directory(file) ) {
            FileNode *node = new FileNode();
            node->setName(file.substr(file.find_last_of(boost::filesystem::path::preferred_separator) + 1, file.length()));
            node->setType('d');
            node->setParentNode(currentPath);

            currentPath->sibling.push_back(*node);

            BOOST_LOG_TRIVIAL(info) << "subFolderName : " << node->getName().string() << " : " << node;

            constructChildPath(node);
        }

    ...
    ...
}
Since the string argument has been removed, the caller doesn't require to pass down the inputPath anymore and also the searchPath is no longer there. The caller has been trimmed down as below:
FileNode* FileBot::loadLocalFS(boost::filesystem::path inputPath)
{
    constructChildPath(constructParentPath(inputPath));

    return localHome;
}
Since searchPath has already taken away, this will have a major impact on constructParentPath() as well, thus I have the following logic append to the end of the method:
FileNode* FileBot::constructParentPath(boost::filesystem::path inputPath, FileListType listType)
{
    ...

    if( listType == local ) {
        localHome = parent;
        return localHome;
    }
    else {
        remoteHome = parent;
        return remoteHome;
    }

//    return searchPath;
}

No comments: