C++ looking for an alternative for this code that isnt repetitive.

Question

Hello There! I am *back*. I haven't been on this platform for quite a while now, but nothing has changed i see

So, I am making a CLI Terminal project in C++ (full code at https://www.github.com/PCPPTech/-F00L.)

```

for(const string& element : folders_withDot)

    inorder.push_back(element);

  for(const string& element : folders)

    inorder.push_back(element);

  for(const string& element : files_withDot)

    inorder.push_back(element);

  for(const string& element : files)

    inorder.push_back(element);
```

here, I iterate thru all the previous string vectors
Here is the code before that:
```

vector<string> folders_withDot, folders, files_withDot, files, inorder; // creating every list where we will store certain stuff in 1 line

  for(const auto& entry : fs::directory_iterator(path)) { // iterate thru the given path

    string filename = entry.path().filename().string(); // This is our readable filename

    // first = Folders starting with .

    if (filename.starts_with(".") && fs::is_directory(entry.path())) // if it starts with a dot and is a directory

      folders_withDot.push_back(filename);                           // then it belongs in the folder_withDot string vector.

    // second = Ordinary Folders (not starting with .)

    if (fs::is_directory(entry.path()) && !(filename.starts_with("."))) // if it is a directory and it doesn't start with a dot

      folders.push_back(filename);                                      // then it belongs in the normal folders string vector

   

    // third = Files starting with a .

    if (!(fs::is_directory(entry.path())) && filename.starts_with(".")) // if it isn't a directory but does start with a dot

      files_withDot.push_back(filename);                                // then it belongs in the files_withDot string vector

    // fourth = files that dont start with a dot.

    if (!(fs::is_directory(entry.path())) && !(filename.starts_with("."))) // if it isn't a directory and it doesnt start with a dot

      files.push_back(filename);                                           // then it belongs in the normal files string vector.

    // iterate thru each one... this is gonna be so so so slow...

    // idk how to iterate thru 4 arrays in 1 for loop without knowing their length for sure

  }
```
I tried to comment everything thoroughly.
So I iterated thru all the sorted string vectors and put them inorder in the string vector `inorder`.

So, my problem is that the first code i provided is really repetitive and I think there is a better way to do it. Maybe nested loops? I dont know.

If you have an idea, Please share it here, I appreciate it! ^^

Answer 1

iterate thru each one... this is gonna be so so so slow...

Is it actually slow? Have you done any measurements on how long it takes to run through that piece of code? Have you done a performance analysis to find out why it is slow? What part of the code makes it slow?

You have to iterate through the files anyway; you cannot avoid that. What you could avoid is doing the same thing multiple times.

E.g. fs::is_directory(entry.path()) is called 4 times. You could just store it in a variable inside your loop once, and not evaluate this expression 4 times in your code.

Same for filename.starts_with(".").

Anything that you use multiple times could be stored in a variable.

Something like this (I haven't made sure the code compiles, though!)

    for(const auto& entry : fs::directory_iterator(path))
    {
        // Cached values as they will be used multiple times
        auto path = entry.path();
        string filename = path.filename().string();
        bool startsWithDot = filename.starts_with(".");
        bool isDirectory = fs::is_directory(path);

        if (isDirectory)
        {
            if (startsWithDot)
                folders_withDot.push_back(filename);
            else
                folders.push_back(filename); 
        }
        else // !isDirectory
        {
            if (startsWithDot)
                files_withDot.push_back(filename); 
            else
                files.push_back(filename);
        }
    }

Comments

Hello! Thank you for your answer. I thought it was inefficient and it could be faster. Also I don't use variable to store those because it helps me navigate thru my code easily and I won't have to remember a bunch of variable names. Also I hate thinking about what to call variables. I know it might be longer but I like it that way.

Thanks for answering!

---

Ah, yeah, the good old problem of naming variables. That's one of the most difficult parts of writing well-maintainable code. Names are sometimes utterly useless (e.g., "tmp" or "a" or even "list" would not help to understand what is stored inside), sometimes straight misleading (e.g., calling something a "customerList", but in fact it's not of a type List, but it's an Array).

Naming functions and classes can also be difficult.

And there's also maintainability. You can call the collection of customers "customerArray", but if you change it to a List, instead of an actual array, then either the name becomes misleading, or you'd need to rename the variable to "customerList". Best practice is just to call them "customers" and not encode the type in the variable. This would be more pronounced with something like "int32_t intNumCustomers", then you later refactor it to "uint32_t", and now you'd have to rename it to "uintNumCustomers". Again, best practice is not to include the type in the name, just call it "numCustomers" or something similar.

Answer 2

through that piece of code? Have you done a performance analysis to find out why it is slow? What part of the code makes it slow?

You have to iterate through the files anyway; you cannot avoid that. What you could avoid is doing the same thing multiple times.

E.g. fs::is_directory(entry.path()) is called 4 times. You could just store it in a variable inside your loop once, and not evaluate this expression 4 times in your code.

Same for filename.starts_with(".").

Anything that you use multiple times could be stored in a variable.

Something like this (I haven't made sure the code compiles, though!)

    for(const auto& entry : fs::directory_iterator(path))
    {
        // Cached values as they will be used multiple times
        auto path = entry.path();
        string filename = path.filename().string();
        bool startsWithDot = filename.starts_with(".");
        bool isDirectory = fs::is_directory(path);

        if (isDirectory)
        {
            if (startsWithDot)
                folders_withDot.push_back(filename);
            else
                folders.push_back(filename); 
        }
        else // !isDirectory
        {
            if (startsWithDot)
                files_withDot.push_back(filename); 
            else
                files.push_back(filename);
        }
    }

Answer 3

We can consider creating a function that takes a collection as input and pushes its elements into  inorder , then call that function for each collection.

Answer 4

你比我強！ ！ ！ ！但是我建議問AI喔!

Answer 5

vector<vector<string>*> all_vectors = { &folders_withDot, &folders, &files_withDot, &files };

for (auto vec_ptr : all_vectors) {
    inorder.insert(inorder.end(), vec_ptr->begin(), vec_ptr->end());
}