1
\$\begingroup\$

I am trying to convert filesystem paths to a tree.

I have following paths

> /path1/path2/path3 
> /path1/path2/path3/cell_lists
> /path1/path2/path3/path4 
> /path1/path2/path3/path4/ALLCELLS
> /path1/path2/path3/path4/ALLCELLS/cells
> /path1/path2/path3/path4/ALLCELLS/cells/field
> /path1/path2/path3/path4/ALLCELLS/cells/field2

which would like to convert into hierarchical path and consider as tree

- path1
  -- path2
     -- path3
        |- path4
        |  -- all_cells
        -- cell_lists

Therefore I've written the following code. I think it can be improved for traversal or storing data.

#include<iostream>
#include<fstream>
#include<vector>
#include<boost/thread/thread.hpp>
#include<boost/filesystem.hpp>
#include<boost/tokenizer.hpp>
#include <boost/algorithm/string.hpp>
#include "subprocess.h"
#include "Command.h"
namespace fs = boost::filesystem;

/*
 * Traverse directory till maxdepth
 * return path
 */
std::vector<std::string> traverse_dir() {
    fs::path p("path");
    int maxdepth = 3;
    std::vector<std::string> traverse_dir;
    fs::recursive_directory_iterator end_iter;
    boost::system::error_code dir_error;
    for(fs::recursive_directory_iterator begin(p, dir_error); begin != end_iter; ++begin) {
        try {
            if(dir_error.value()) {
                std::cout<<"The directory has error"<<dir_error.message()<<std::endl;
                continue;
            }
            if(fs::is_symlink(begin->path())) {
                    begin.no_push();
            }
            if(fs::is_directory(begin->status())){
                if(begin.level()== maxdepth)
                    begin.no_push();
                traverse_dir.push_back(begin->path().string());
            }
        }catch(fs::filesystem_error &ex) {
            std::cout<<"Boost Filesystem Error"<< ex.what()<<std::endl;
        }
        catch(const std::exception &ex){
            std::cout<<"Standard Exception"<<ex.what()<<std::endl;
        }
    }
    return traverse_dir;
}

struct Dirinfo {
    uint64_t dir_size;
    std::string owner;
    std::string dir_name;
    std::string dir_path;
    std::string f_time;
    std::vector<Dirinfo*> list;
    Dirinfo *next;
};

class Dirmapping {
    private:
        Dirinfo *root = nullptr;
        std::vector<std::string> dirs;
    public:
        Dirmapping() = default;
        Dirmapping(const std::vector<std::string> &dir_name);
        void createmap();
        void printchild(Dirinfo *child, std::ofstream &outfile, std::string &line);
        void printmap(Dirinfo *root);
};

Dirmapping::Dirmapping(const std::vector<std::string> &dirs): dirs(dirs) {}


void Dirmapping::createmap() {
    for(auto it = this->dirs.begin();  it != dirs.end(); ++it) {
        std::cout<<"processing path "<<*it<<std::endl;
        fs::path insert_path(*it);
        Dirinfo *dirinfo = new Dirinfo;
        dirinfo->dir_path = *it;
        dirinfo->dir_name = insert_path.filename();
        dirinfo->f_time = std::to_string(fs::last_write_time(insert_path));
        dirinfo->next = nullptr;
        dirinfo->owner = "";
        dirinfo->dir_size = 0;
        if (root == nullptr) {
            root = dirinfo;
        }
        else {
            Dirinfo *temp = root;
            bool found_root = false;
            bool found_child_root = false;
            while(temp != nullptr) {
                fs::path p(temp->dir_path);
                fs::path compare_path(*it);
                if(fs::equivalent(compare_path.parent_path(), fs::path(temp->dir_path))){
                    found_root = true;
                    break;
                }
                for(auto it_child = temp->list.begin(); it_child != temp->list.end(); ++it_child){
                    std::string compare_path_string = compare_path.parent_path().string();
                    if(compare_path_string.compare((*it_child)->dir_path)){
                        (*it_child)->list.push_back(dirinfo);
                        found_child_root = true;
                        std::cout<<"found child node"<<std::endl;
                        break;
                    }
                    compare_path = compare_path.parent_path();
                }

                if(found_root || found_child_root)
                    break;
                if(temp->next == nullptr)
                    break;
                temp= temp->next;
            }
            if(found_root){
                temp->list.push_back(dirinfo);
            }
            else if(!found_child_root){
                temp->next = dirinfo;
            }
        }
    }
    printmap(root);
}

void Dirmapping::printchild(Dirinfo *temp, std::ofstream &outfile, std::string& line) {
    outfile<<temp->dir_path<<"  "<<"\n";
    for(std::vector<Dirinfo*>::iterator it = temp->list.begin(); it != temp->list.end(); ++it){
        outfile<<line<<(*it)->dir_path<<"\n";
        if((*it)->list.size())
            line += "====";
            printchild(*it, outfile, line);
            line = "";
    }
}

void Dirmapping::printmap(Dirinfo *root) {
    Dirinfo *temp = root;
    std::ofstream outfile("dir_structure");
    std::string line = "=";
    while(temp != nullptr){
        uint64_t size = 0;
        outfile<<"Root:"<<temp->dir_path<<"\n\n";
        if(temp->list.size()){
            printchild(temp, outfile, line);
        }
        temp = temp->next;
    }
    outfile.close();
}

int main() {
    std::vector<std::string> dir_names = traverse_dir();
    Dirmapping dirmap(dir_names);
    dirmap.createmap();
    return 0;
}
\$\endgroup\$
0
2
\$\begingroup\$

traverse_dir could take a parameter that specifies the maxdepth and the starting point. It becomes:

std::vector<std::string> traverse_dir(const fs::path *begin, const unsigned maxdepth) { 
    //...

For maximum configurability, you should consider putting in configuration options about following symlinks.

Note that I directly changed maxdepth from int to unsigned int. After all negative maximum depth really makes little sense. I was also considering size_t, but decided against it, since it's semantically incorrect in that place.

it is becoming of code that traverses directories to return instances of fs::path. Repeat after me: A path is not a string and a string is not a path! Strings are not generally a useful replacement for path and they shouldn't be used as one. Then again Boost.Filesystem does exactly that, so it might be fine either way.

Your use of iterators seems somewhat fishy, but reading the boost reference it seems like that's the intended usage code. As someone not regularly writing c++ it feels weird that the default construction of an iterator yields a "beyond end" iterator.
I'd also name the iterator it or itr, not begin.

Your documentation does not state that traverse_dir will only return directories, but never files. That's a bit ... unintuitive. I'd personally also prefer to handle the max_depth seperately from the pushing logic. It's not relevant whether the currently examined path is a directory for the max_depth logic. Consider the following code:

for (fs::recursive_directory_iterator it(p, dir_error); it != end; ++it) {
    try {
        if (dir_error.value()) {
            std::cout << "An error occured when traversing " << p 
                      << ": " << dir_error.messsage() << std::endl;
        }
        if (it.level() >= maxdepth || fs::is_symlink(it->path())) {
            it.no_push(); //skip symlinks and don't go beyond maxdepth
        }
        if (fs::is_directory(it->status()) {
            traverse_dir.push_back(it->path().string());
        }
    } // catches and close for-loop

This code makes a few things a bit more obvious. One could even go so far as to separate the symlink-check and the depth check.
Note that I also added a space around all binary operators and reworded the error message a bit to include more details.

On a related note: Please be consistent in how you place catches. the first one is on the same line as the closing brace, but the second one is on a new line... Generally speaking consistency is king.

Lastly considering that Boost.Filesystem does not access the filesystem for instances of path you might as well return a std::vector<fs::path> and get rid of the call to .string().

\$\endgroup\$
2
  • \$\begingroup\$ What's so weird about past-the-end interators? The STL uses them as well. \$\endgroup\$ – yuri Jun 18 '18 at 9:34
  • \$\begingroup\$ @yuri but it exposes them as it.end() iirc \$\endgroup\$ – Vogel612 Jun 18 '18 at 9:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.