2
\$\begingroup\$

I created a program where I was constantly needing o know the number of files in a folder, number of folders, and getting these file names into a vector, so I made this wrapper because I am planning on creating a program that will as well constantly need the informations above mentioned.

mydirent.h

#ifndef MYDIRENT_H_INCLUDED
#define MYDIRENT_H_INCLUDED

#include <string>
#include <vector>

namespace dir
{
    class DirentWrap
    {
    private:
        DIR *dp;
        struct dirent *ep;
        DirentWrap(){}
    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);
    };
}

#endif // MYDIRENT_H_INCLUDED

mydirent.cpp

#include "mydirent.h"
#include <dirent.h>
#include <iostream>
#include <sys/stat.h>

using namespace std;

using namespace dir;

class MyException : public exception
{
    virtual const char* what() const throw()
    {
        return "error openning folder";
    }
} ex;

int DirentWrap::number_of_files_in_directory(string file_path)
{
    // a contagem vai começar apartir de -2 pk em todas as pastas parece sempre haver 2 ficheiros desconhecidos pra mim ( . , .. )
    int counter = 0;

    string secure_file_path = file_path + "\\";

    DirentWrap dr;

    dr.dp = opendir(secure_file_path.c_str());

    struct stat s;

    if (dr.dp != nullptr)
    {
        while ((dr.ep = readdir(dr.dp)))
        {
            string path = secure_file_path + dr.ep->d_name;

            stat(path.c_str(), &s);

            if(s.st_mode & S_IFREG)
                ++counter;
        }
    }
    else
    {
        closedir(dr.dp);

        throw ex;
    }

    closedir(dr.dp);

    return counter;
}

int DirentWrap::number_of_folders_in_directory(string file_path)
{
    // a contagem vai começar apartir de -2 pk em todas as pastas parece sempre haver 2 ficheiros desconhecidos pra mim ( . , .. )
    int counter = -2;

    string secure_file_path = file_path + "\\";

    DirentWrap dr;

    dr.dp = opendir(secure_file_path.c_str());

    struct stat s;

    if (dr.dp != nullptr)
    {
        while ((dr.ep = readdir(dr.dp)))
        {
            string path = secure_file_path + dr.ep->d_name;

            stat(path.c_str(), &s);

            if(s.st_mode & S_IFDIR)
                ++counter;
        }
    }
    else
    {
        closedir(dr.dp);

        throw ex;
    }

    closedir(dr.dp);

    return counter;
}

void DirentWrap::files_in_directory(string file_path, vector<std::string>& files)
{
    string secure_file_path = file_path + "\\";

    DirentWrap dr;

    dr.dp = opendir(secure_file_path.c_str());

    struct stat s;

    if (dr.dp != nullptr)
    {
        while ((dr.ep = readdir(dr.dp)))
        {
            string path = secure_file_path + dr.ep->d_name;

            stat(path.c_str(), &s);

            if(s.st_mode & S_IFREG)
            {
                files.push_back(dr.ep->d_name);
            }
        }
    }
    else
    {
        closedir(dr.dp);

        throw ex;
    }

    closedir(dr.dp);
}

void DirentWrap::folders_in_directory(string file_path, vector<std::string>& folders)
{
    int counter = 0;

    string secure_file_path = file_path + "\\";

    DirentWrap dr;

    dr.dp = opendir(secure_file_path.c_str());

    struct stat s;

    if (dr.dp != nullptr)
    {
        while ((dr.ep = readdir(dr.dp)))
        {
            ++counter;

            if (counter > 2)
            {
                string path = secure_file_path + dr.ep->d_name;

                stat(path.c_str(), &s);

                if(s.st_mode & S_IFDIR)
                {
                    folders.push_back(dr.ep->d_name);
                }
            }
        }
    }
    else
    {
        closedir(dr.dp);

        throw ex;
    }

    closedir(dr.dp);
}
\$\endgroup\$
  • \$\begingroup\$ What if the directory entries have already been retrieved, and I just want to get the list of names multiple times? There is no caching mechanism in your code. \$\endgroup\$ – PaulMcKenzie Dec 29 '19 at 4:48
  • \$\begingroup\$ @PaulMcKenzie I actually don't know about caching, I mean, I know what is it and what it is used for but that is it, I don't know how to implemement it and when it is needed (as in this wrapper). Any suggestion on how to get started? \$\endgroup\$ – HBatalha Dec 29 '19 at 9:25
  • 1
    \$\begingroup\$ Please see How do I ask a good question?, in particular the Titling your question section. \$\endgroup\$ – S.S. Anne Dec 29 '19 at 17:02
  • \$\begingroup\$ I think you can do it without developing a C program. I think using the operating system shell is enough (you can use a cron or task schedule if you need them to run at intervals). \$\endgroup\$ – Juan Carlos Guibovich Jan 1 at 10:11
5
\$\begingroup\$
  • DRY.

    The number_of_files_in_directory and number_of_folders_in_directory methods are almost identical. Factor out the common code into a private method, say number_of_entities_in_directory, and rewrite public methods as

    int DirentWrap::number_of_files_in_directory(string file_path)
    {
        return number_of_entities_in_directory(path, S_IFREG);
    }
    
    int DirentWrap::number_of_folders_in_directory(string file_path)
    {
        return number_of_entities_in_directory(path, S_IFDIR) - 2;
    }
    
  • Portability.

    • "\\" as a path separator works for Windows only. Prefer "/", which works for both Linux and Windows (the backslash is only required in cmd.exe).

    • folders_in_directory assumes that . and .. always appear first. readdir does not guarantee it.

    • Consider using std::experimental::filesystem library.

  • Error checking.

    • stat may fail. readdir may fail (if so it would return NULL, and you need to test errno, rather than blindly breaking a loop).

    • Similarly, the exception you throw on opendir failure loses the important information, namely why opendir failed. Provide errno or strerror(errno).

  • Why class?

    class DirentWrap does not have any state. There is no reason to have it. Its methods should be made free functions, with dp and ep being their local variables.

\$\endgroup\$
  • \$\begingroup\$ Under Portability: the . and .. always appear in the folders I read, I don't know why and if they mean something. Under Error checking: How would I implement the test errno with stat and readdir. I tried something like this: if (readdir(dp)) while ((ep = readdir(dp))), don't know if it is correct. Why Class? When using any of those functions I wanted to tell just by readding the code that the function comes from a simple wrap and not a header like dirent.h itself . Thanks for your review, I really appreciate it. Notes taken, changes made. \$\endgroup\$ – HBatalha Dec 30 '19 at 16:41
  • \$\begingroup\$ the code provided in the Dry section doesn't work, throws an error stating that it can't call the function without an object \$\endgroup\$ – HBatalha Dec 30 '19 at 19:46
5
\$\begingroup\$
  1. Don't use a second instance of the class just to store variables. Further, don't even use member variables for things local to a function.
  2. In this case, caching would be making one, probably private, method to read the directory listing and populate internal variables with the data. This would allow all four current methods to just ensure the data has been loaded, and then return things from the internal variables. This allows you, for instance, to fetch the number of files and know that when you fetch the list of files, you will get the same number. This may entail adding a reload() method so that you can take a fresh look.
  3. Your readdir() function may return the file type. If it does, use it. This turns an O(n*n) operation into an O(n) operation. (If you don't follow that, try using a directory with 100,000 files in it.)
  4. It is probably unwise to assume that "." and ".." will be the first two entries in a directory.
  5. if(s.st_mode & S_IFDIR) is a mistake. The correct usage is: if((s.st_mode & S_IFMT) == S_IFDIR) Alternatively, you can write if(S_ISDIR(s.st_mode)) The same applies to the other tests.
\$\endgroup\$
  • \$\begingroup\$ I don't understand 1. In 2 I don't know how to implement a caching mechanism yet so I don't follow your suggestion very well. I don't follow 3, on 4 I don't know why the . and .. always appear, because they were always showing up so I assumed it they would always . \$\endgroup\$ – HBatalha Dec 30 '19 at 17:23
  • \$\begingroup\$ 1: You should not make an instance of your class in a method just to use it's variables. [I admit to not noticing your methods are static.] Additionally, neither of the variables couldn't have just been local variables. \$\endgroup\$ – David G. Dec 31 '19 at 1:34
  • \$\begingroup\$ 2: Learn about caching. Caching basically means calculating the values once and remembering them for later use. It thus trades off speed for memory. \$\endgroup\$ – David G. Dec 31 '19 at 1:36
  • \$\begingroup\$ 3: Some modern readdir() functions will tell you if the entry is a file or a directory. If it can, you don't need to stat() the file. stat()ing the file require looking it up, which typically takes log(n) time for a directory with n entries. Avoiding that lookup can be a significant performance boost. \$\endgroup\$ – David G. Dec 31 '19 at 1:38
  • \$\begingroup\$ 4: Yes, they (normally) always show up because . is the current directory and .. is the parent directory. But I don't believe they are ever guaranteed to be the first two entries. A filesystem could theoretically sort the entries for you,and put file ! first. In theory, there could be an operating system that doesn't include these entries, or that names them differently. (It might not be posix compliant, but that doesn't mean your code has to fail.) \$\endgroup\$ – David G. Dec 31 '19 at 1:44
3
\$\begingroup\$

Use (inspiration from) C++17's std::filesystem

C++17 introduced std::filesystem, which provides portable functions to query things like which files there are in a directory. If you can use C++17, then make use of these functions. If you need to be compatible with older C++ standards, then at least try to emulate what std::filesystem does as much as possible, including functions names and the general behavior.

For example, std::filesystem provides iterators to loop over all elements in a directory. This avoids having to build up a vector, which might be inefficient if you are only going to use it to find an element in it and then discard it.

There is more to filesystems than files and directories

There can be things in a directory that are neither normal files nor other directories, for example:

  • symbolic links
  • block devices
  • character devices
  • fifos
  • sockets

The above is a list of things you can find on Linux and Mac OS X platforms, but even Windows has things like symbolic links and junctions.

Your code currently checks whether something is a regular file, but many of the above filesystem elements can be accessed as if they are regular files. So think careful about what semantics you want your functions to have.

\$\endgroup\$

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.