2
\$\begingroup\$

This is a wrap that queries the number of files and folder from a given directory, also list the folders and files into a vector. I have posted another wrap in this discussion that do the same things and following some really good suggestions I created another this time using filesystem.

The filesystem I used is this implementation I found on GitHub because the filesystem I tried to use in codeblocks kept throwing errors even the filesystem from GCC 9.2.

I didn't implement a cache mechanism because I don't know how to yet.

wrap.h

#ifndef WRAP_H_INCLUDED
#define WRAP_H_INCLUDED

#include <string>
#include <vector>

#include <ghc/filesystem.hpp>

namespace dir
{
class Wrap
{
private:
    Wrap() {}

    enum mode {FOLDER = 0, FILE_};

    int number_of_entities_in_directory(std::string file_path, mode type_);
    std::vector<std::string> entities_in_directory(std::string file_path, mode type_);

    int number_of_entities_in_directory_recursively(std::string file_path, mode type_);
    std::vector<std::string> entities_in_directory_recursively(std::string file_path, mode type_);

public:
    static int number_of_files_in_directory(std::string file_path);
    static int number_of_folders_in_directory(std::string file_path);
    static void files_in_directory(std::string file_path, std::vector<std::string>& files);
    static void folders_in_directory(std::string file_path, std::vector<std::string>& folders);

    static int number_of_files_in_directory_recursively(std::string file_path);
    static int number_of_folders_in_directory_recursively(std::string file_path);
    static void files_in_directory_recursively(std::string file_path, std::vector<std::string>& files);
    static void folders_in_directory_recursively(std::string file_path, std::vector<std::string>& folders);

};
}

#endif // WRAP_H_INCLUDED

wrap.cpp

#include "wrap.h"

#include <ghc/filesystem.hpp>

using namespace ghc::filesystem;

using namespace std;

using namespace dir;

int Wrap::number_of_entities_in_directory(string file_path, mode type_)
{
    int counter = 0;

    string secure_file_path = file_path + "/";

    if ( ! exists(secure_file_path))
    {
        throw runtime_error("directory does not exist");
    }

    if(ghc::filesystem::is_empty(secure_file_path))
        return 0;

    directory_iterator it (secure_file_path);
    directory_iterator endit;

    while(it != endit)
    {
        if(type_ == FILE_ && is_regular_file(*it))
            ++counter;
        else if(type_ == FOLDER && is_directory(*it))
            ++counter;

        ++it;
    }

    return counter;
}

vector<string> Wrap::entities_in_directory(string file_path, mode type_)
{
    vector<string> entities;

    string secure_file_path = file_path + "/";

    if ( ! exists(secure_file_path))
    {
        throw runtime_error("directory does not exist");
    }

    directory_iterator it(secure_file_path);
    directory_iterator endit;

    if( ! ghc::filesystem::is_empty(secure_file_path))
        while (it != endit)
        {
            if(type_ == FILE_ && is_regular_file(*it))
                entities.push_back(it->path().filename());
            else if(type_ == FOLDER && is_directory(*it))
                entities.push_back(it->path().filename());

            ++it;
        }

    return entities;
}

int Wrap::number_of_entities_in_directory_recursively(string file_path,
        mode type_)
{
    int counter = 0;

    string secure_file_path = file_path + "/";

    if ( ! exists(secure_file_path))
    {
        throw runtime_error("directory does not exist");
    }

    if(ghc::filesystem::is_empty(secure_file_path))
        return 0;

    recursive_directory_iterator it (secure_file_path);
    recursive_directory_iterator endit;

    while(it != endit)
    {
        if(type_ == FILE_ && is_regular_file(*it))
            ++counter;
        else if(type_ == FOLDER && is_directory(*it))
            ++counter;

        ++it;
    }

    return counter;
}

vector<string> Wrap::entities_in_directory_recursively(string file_path,
        mode type_)
{
    vector<string> entities;

    string secure_file_path = file_path + "/";

    if ( ! exists(secure_file_path))
    {
        throw runtime_error("directory does not exist");
    }

    recursive_directory_iterator it(secure_file_path);
    recursive_directory_iterator endit;

    if( ! ghc::filesystem::is_empty(secure_file_path))
        while (it != endit)
        {
            if(type_ == FILE_ && is_regular_file(*it))
                entities.push_back(it->path().filename());
            else if(type_ == FOLDER && is_directory(*it))
                entities.push_back(it->path().filename());

            ++it;
        }

    return entities;
}

int Wrap::number_of_files_in_directory(string file_path)
{
    if(file_path.empty())
        return 0;

    Wrap dr;
    return dr.number_of_entities_in_directory(file_path, FILE_);
}

int Wrap::number_of_folders_in_directory(string file_path)
{
    if(file_path.empty())
        return 0;

    Wrap dr;
    return dr.number_of_entities_in_directory(file_path, FOLDER);;
}

void Wrap::files_in_directory(string file_path, vector<std::string>& files)
{
    if(file_path.empty())
        return;

    Wrap dr;
    files = dr.entities_in_directory(file_path, FILE_);
}

void Wrap::folders_in_directory(string file_path, vector<std::string>& folders)
{
    if(file_path.empty())
        return;

    Wrap dr;
    folders = dr.entities_in_directory(file_path, FOLDER);
}


int Wrap::number_of_files_in_directory_recursively(string file_path)
{
    if(file_path.empty())
        return 0;

    Wrap dr;
    return dr.number_of_entities_in_directory_recursively(file_path, FILE_);
}

int Wrap::number_of_folders_in_directory_recursively(string file_path)
{
    if(file_path.empty())
        return 0;

    Wrap dr;
    return dr.number_of_entities_in_directory_recursively(file_path, FOLDER);;
}

void Wrap::files_in_directory_recursively(string file_path,
        vector<std::string>& files)
{
    if(file_path.empty())
        return;

    Wrap dr;
    files = dr.entities_in_directory_recursively(file_path, FILE_);
}

void Wrap::folders_in_directory_recursively(string file_path,
        vector<std::string>& folders)
{
    if(file_path.empty())
        return;

    Wrap dr;
    folders = dr.entities_in_directory_recursively(file_path, FOLDER);
}

main.cpp

#include <iostream>
#include <vector>
#include <wrap.h>

using namespace dir;
using namespace std;

int main()
{
    string path = "D:/Music/";

    vector<string> vec;

    Wrap::folders_in_directory(path, vec);

    for(auto i : vec)
        cout << i << endl;

    return 0;
}

I created the wrapper as a static library project and added the header file to the include folder, hence the line:

#include <wrap.h>
\$\endgroup\$
  • \$\begingroup\$ Can you provide some test code that exercises the wrap and demonstrates how to use it in code. \$\endgroup\$ – pacmaninbw Dec 31 '19 at 1:42
  • \$\begingroup\$ @pacmaninbw done \$\endgroup\$ – HBatalha Dec 31 '19 at 1:58
2
\$\begingroup\$

Avoid using too much namespace

You risk having identical names mapped onto each other. Either be more selective in what you are using, or create short aliases if you want to avoid typing long namespace names, like:

namespace fs = ghc::filesystem;

You already ran into this problem with is_empty().

Don't #include headers you don't need

In wrap.h, you #include <ghc/filesystem.hpp>, but nothing in that header file uses it. You should remove it, unless you plan to make the functions take ghc::filesystem::path's as parameters instead of std::strings.

Use enum class where possible

A regular enum is not very type-safe. Prefer an enum class.

Be consistent in how you return results

Your private functions just return std::vector<std::string>, but the public functions take a reference to a std::vector<std::string>. Why is there a difference? Since the vector of strings is the result, just return it everywhere.

Make the private member functions static

There is no reason for the private member functions not to be static in your class. But, then you'll realize that everything is static, which brings us to:

There is no need to use a class

You are not storing any state in class Wrap, and all functions are or can be made static. So there is no reason to use a class here at all. You could move all the public member functions out of the class into namespace dir. The private member functions don't have to appear in wrap.h at all anymore.

Why the underscores?

Why write FILE_ and type_? Unless they are macros you should be able to declare those symbols in your own classes and namespaces without the trailing underscore.

Avoid redundant tests for empty directories

There is no need to test for the directory being empty; your loops handle empty directories perfectly fine. You are adding unnecessary overhead in the common case where a directory is not empty, and if it is empty, your loops will just do nothing, so there is no performance penalty.

Use range-for where appropriate

You can use range-for to iterate over a directory, just like an STL container. So for example, in Wrap::number_of_entities_in_directory(), you can write:

for(auto entity: directory_iterator(secure_file_path)) {
    if(type == mode::FILE && is_regular_file(entity))
        ++counter;
    else if(type == mode::FOLDER && is_directory(entity))
        ++counter;
}

Consider using STL algorithms

Instead of writing your own loops, you could use STL algorithms. In fact, the whole function could be rewritten as:

#include <algorithm>
namespace fs = ghc::filesystem;
...
int Wrap::number_of_entities_in_directory(string file_path, mode type) {
    directory_iterator it(file_path);
    return std::count_if(fs::begin(it), fs::end(it), [type](const directory_entry &ent){
        return type == FOLDER ? is_directory(ent) : is_regular_file(ent);
    });
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ [Be consistent in how you return results] Do you mean that instead of this: void Wrap::files_in_directory(std::string file_path, vector<std::string>& files) I should have it like this: vector<std::string>Wrap::files_in_directory(std::string file_path) \$\endgroup\$ – HBatalha Jan 1 at 20:11
  • \$\begingroup\$ Yes, that is what I meant. \$\endgroup\$ – G. Sliepen Jan 1 at 20:14
  • \$\begingroup\$ Actually that was the first way I implemented that function but then I changed it into how it is now, I just prefer it like this \$\endgroup\$ – HBatalha Jan 1 at 20:21
  • \$\begingroup\$ [There is no need to use a class] I wanted to put all the function definition into a cpp file and have the header with only declarations of those functions. Don't know at what extent it is worth though \$\endgroup\$ – HBatalha Jan 1 at 20:25
  • \$\begingroup\$ That's unrelated to whether you use a class or not. You can still have the declarations in the header file outside a class, and have the definitions in a cpp file. \$\endgroup\$ – G. Sliepen Jan 1 at 20:27

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.