C++ Sorting Filenames In A Directory

Question

I wanted to have some advice about the code I have.

I managed to get what I wanted done, but I do not think it is the "proper" way of doing it in the programmers' world.

Could you help me improve the code by any means and also if there are any better ways of doing this please share them as well.

I have files named in the format:

501.236.pcd

501.372.pcd

...

612.248.pcd etc.

I wanted to put the filenames in ascending order according to the filenames using C++.

This is the code I use:

#include <string>
#include <iostream>
#include <boost/filesystem.hpp>
#include <sstream>

using namespace std;
using namespace boost::filesystem;

int main()
{
    vector <string> str,parsed_str;
    path p("./fake_pcd");
    string delimiter = ".";
    string token,parsed_filename;
    size_t pos = 0;
    int int_filename;
    vector <int> int_dir;

    //insert filenames in the directory to a string vector
    for (auto i = directory_iterator(p); i != directory_iterator(); i++)
    {
        if (!is_directory(i->path())) //we eliminate directories in a list
        {
        str.insert(str.end(),i->path().filename().string());
        }
        else
            continue;
    }

    //parse each string element in the vector, split from each delimiter 
    //add each token together and convert to integer
    //put inside a integer vector 
    parsed_str = str;
    for (std::vector<string>::iterator i=parsed_str.begin(); i != parsed_str.end(); ++i)
    {
        cout << *i << endl;
        while ((pos = i->find(delimiter)) != string::npos) {    
        token = i->substr(0,pos);
        parsed_filename += token;
        i->erase(0, pos + delimiter.length());
    }

    int_filename = stoi(parsed_filename);
    int_dir.push_back(int_filename);

    parsed_filename = "";
    }

    cout << endl;

    parsed_str.clear();

    sort(int_dir.begin(), int_dir.end());

    //print the sorted integers
    for(vector<int>::const_iterator i=int_dir.begin(); i != int_dir.end(); i++) {
        cout << *i << endl;
    }

    //convert sorted integers to string and put them back into string vector
    for (auto &x : int_dir) {
        stringstream ss;
        ss << x;
        string y;
        ss >> y;
        parsed_str.push_back(y);
    }

    cout << endl;

    //change the strings so that they are like the original filenames  
    for(vector<string>::iterator i=parsed_str.begin(); i != parsed_str.end(); i++) {
        *i = i->substr(0,3) + "." + i->substr(3,3) + ".pcd";
        cout << *i << endl;
    }


}

This is the output, first part is in the order the directory_iterator gets it, the second part is the filenames sorted in integers, and the last part is where I change the integers back into strings in the original filename format.

612.948.pcd
612.247.pcd
501.567.pcd
501.346.pcd
501.236.pcd
512.567.pcd
613.008.pcd
502.567.pcd
612.237.pcd
612.248.pcd

501236
501346
501567
502567
512567
612237
612247
612248
612948
613008

501.236.pcd
501.346.pcd
501.567.pcd
502.567.pcd
512.567.pcd
612.237.pcd
612.247.pcd
612.248.pcd
612.948.pcd
613.008.pcd

Answer 1

If you have access to C++17, you can use std::filesystem instead of boost::filesystem.

This is gonna cause a ton of problems by obscuring the source of names used in the code and introducing name clashes:

using namespace std;
using namespace boost::filesystem;

Qualify names from std instead, and maybe namespace fs = std::filesystem;.

---

Overall, the main function is very long and requires a lot of brain energy to understand.

Declaring a lot of variables at the start of a block makes the logic hard to follow:

vector <string> str,parsed_str;
path p("./fake_pcd");
string delimiter = ".";
string token,parsed_filename;
size_t pos = 0;
int int_filename;
vector <int> int_dir;

Some of the variables have sub-optimal names. Also consider accepting the path as an argument for more flexibility:

int main(int argc, char* argv[])
{
    // validate ...
    std::path directory{std::string{argv[1]}};
    // ...
}

This is convoluted:

//insert filenames in the directory to a string vector
for (auto i = directory_iterator(p); i != directory_iterator(); i++)
{
    if (!is_directory(i->path())) //we eliminate directories in a list
    {
    str.insert(str.end(),i->path().filename().string());
    }
    else
        continue;
}

Simplification: (note that directory_iterator is a range by itself)

std::vector<std::string> filenames;
for (const auto& entry : fs::directory_iterator{directory}) {
    if (!entry.is_directory()) {
        filenames.push_back(entry.path().filename().string());
    }
}

Sorry, I gave up trying to understand everything beyond this point.

---

Here's basically how I would write the same code (not tested):

#include <algorithm>
#include <cstdlib>
#include <filesystem>
#include <iostream>
#include <string>
#include <vector>

namespace fs = std::filesystem;

fs::directory_entry parse_args(int argc, char* argv[])
{
    if (argc != 2) {
        std::cerr << "Usage: " << argv[0] << " <directory>\n";
        std::exit(EXIT_FAILURE);
    }

    fs::directory_entry directory{argv[1]};
    if (!directory.is_directory()) {
        std::cerr << '\'' << argv[1] << "' is not a directory\n";
        std::exit(EXIT_FAILURE);
    }
    return directory;
}

int main(int argc, char* argv[])
{
    auto directory = parse_args(argc, argv);

    std::vector<fs::path> filenames;
    for (const auto& entry : fs::directory_iterator{directory}) {
        if (entry.is_regular_file()) {
            filenames.push_back(entry.path().filename());
        }
    }

    std::sort(filenames.begin(), filenames.end(),
              [](const auto& lhs, const auto& rhs) {
                  return lhs.string() < rhs.string();
              });
    for (const auto& file : filenames) {
        std::cout << file.string() << '\n';
    }
}