Tuesday, November 6, 2018

Found a new usage of getLoadFile()

Initially I was thinking to sunset the getLoadFile() as it's just another dump function for debugging purpose, but now I am going to need it in the search process. The original function will accept a FileNode object and then recursively loop through each path until the last object has reached, then convert each object into the URL string.
std::list<string> FileBot::getLoadedFiles(FileNode *fileNode)
{
    std::list<string> mlist;

    if( fileNode == nullptr || fileNode->sibling.size() == 0 )
        return mlist;

    for( FileNodeListType::iterator it = fileNode->sibling.begin(); it != fileNode->sibling.end(); ++it )
    {
        FileNode *n = (&*it);

        if( n->getType() == 'd' ) {
            std::list<string> subList;
            subList = getLoadedFiles(n);

            mlist.insert(mlist.begin(), subList.begin(), subList.end());
        }
        else {
            string filePath = constructPathAddress(n);
            mlist.push_back(filePath);
        }
    }

    return mlist;
}
This was originally used in path construction process to verify the path has correctly loaded into memory by verifying the URL returned. For now, I need this to retrieve all the FileNode objects that park under a particular FileNode object rather than the URL string. Thus, I am converting the list storage to store the FileNode object instead of string:
std::list<FileNode*> FileBot::getLoadedFiles(FileNode *fileNode)
{
    std::list<FileNode*> mlist;

    ...

    for( FileNodeListType::iterator it = fileNode->sibling.begin(); it != fileNode->sibling.end(); ++it )
    {
        ...

        if( n->getType() == 'd' ) {
           ...
           ...
        }
        else {
            mlist.push_back(n);
        }
    }

    return mlist;
}
On the unit testing site, originally I have the following code to verify my desired URL was loaded into the memory. The std::find is a very useful function to lookup a value in the list.
BOOST_AUTO_TEST_CASE(TL_5, *boost::unit_test::precondition(skipTest(false)))
{
    ...

    // verify the file name was captured
    std::list<string> files = fb.getLoadedFiles(root);

    if( std::find(files.begin(), files.end(), "/home/kokhoe/workspaceqt/debug/FolderA/file_1.txt") == files.end() )
        BOOST_TEST(false);
}
Due to the change to object type, the std::find is unusable anymore. But I found a workaround on this, to lookup a value in the object list, that is to create a new functor for this purpose. The URL is now constructed and perform matching inside functor.
struct FileNodeFinder
{
    FileNodeFinder(const std::string &s) : s_(s) {}

    bool operator() (const FileNode* obj)
    {
        FileBotUnderTest fb;

        return fb.constructPathAddress(obj).compare(s_) == 0 ? true : false ;
    }

private:
    const std::string s_;
};
Thus, the new way of doing work is like this:
BOOST_AUTO_TEST_CASE(TL_5, *boost::unit_test::precondition(skipTest(false)))
{
    ...

    // verify the file name was captured
    std::list<FileNode*>::iterator it = find_if(files.begin(),
                                                files.end(),
                                                FileNodeFinder("/home/kokhoe/workspaceqt/debug/FolderA/file_1.txt"));
    FileNode *n = *it;

    if( it == files.end() )
        BOOST_FAIL("nothing were found");
}

Thursday, November 1, 2018

Pretty bad design on the return of constructParentPath()

Not long ago, there was some major change on constructParentPath() in order to maintain 2 different types of request, one for local home path and another one for the remote home path. The FileListType is used to determine whether this function is generated in local home path or remote home path before it get returned. See the code snippet below:
FileNode* FileBot::constructParentPath(boost::filesystem::path inputPath, FileListType listType)
{
    FileNode *parent = NULL;

    ...
    ...

    for( string_split_iterator it = make_split_iterator(path, first_finder(GENERIC_PATH_SEPARATOR, is_equal()));
         it != string_split_iterator();
         ++it)
    {
        ...
        ...

        if( parent == NULL )
            if( listType == local )
                localList.push_back(*node);
            else
                remoteList.push_back(*node);
        else {
            parent->sibling.push_back(*node);
            node->setParentNode(parent);
        }

        ...
        ...
    }

    ...

    if( listType == local ) {
        localHome = parent;
        return localHome;
    }
    else {
        remoteHome = parent;
        return remoteHome;
    }
}
Notice the parent is assigned back to the respective home variable, I was thinking this code might need to rework if there is another third option is coming in. Pretty bad, right?!! Thus, I just feel the return code at the end is redundant. To prevent that from happening in future, I might as well just return the parent and let the caller to decide where the value should assign.
FileNode* FileBot::constructParentPath(boost::filesystem::path inputPath, FileListType listType)
{
    FileNode *parent = NULL;

    ...
    ...

    return parent;
}
As for this change, there are 2 callers on this function. One is loadLocalFS() and another one is loadRemoteFS(). Below are the changes made to adopt this new change:
FileNode* FileBot::loadLocalFS(boost::filesystem::path inputPath)
{
    localHome = constructParentPath(inputPath);
    constructChildPath(localHome);

    ...
}


FileNode* FileBot::loadRemoteFS(vector<string> fileList)
{
    ...

    // load up the abstract path for first entry
    remoteHome = constructParentPath(fileList[0], remote);

    ...
    ...
}