C++ file renaming best style

user_16753585 0
Viewed 129

I have 50 image files which I want to rename.

from: img (1) - Copy.jpg
to: picture_1.jpg

Do you know more elegant way to write it? I came up with this:

#include <iostream>
#include <cstdio>
#include <string>

using namespace std;

int main()
{   
    string oldname = "";
    string newname = "";
    char oldfilename[20];
    char newfilename[30];

    for (int i = 1; i <= 50; i++)
    {       
        oldname = "img ("+ to_string(i) +") - copy.jpg";        // old file name
        newname = "picture_" + to_string(i) + ".jpg";           // new file name    
        strcpy(oldfilename, oldname.c_str());
        strcpy(newfilename, newname.c_str());
        if (rename(oldfilename, newfilename) != 0)
            perror("Error renaming file");
        else
        {
            cout << oldfilename << " renamed successfully to " << newfilename << endl;
        }
    }
    char c;
    cin >> c;   
    return 0;
}
c++
3 Answers

This smells like an x y problem. Why are you reaching for C++ to do this?

If you insist on C++ (not to be confused with C), then you should avoid char arrays and strcpy and friends. In particular, strcpy is very dangerous (at least use strncpy, which can't overrun the buffer).

user_8746600

The code can be improved a great deal by making use of C++17 and C++20 features. In particular, formatting the filename can be done with std::format() (or use the {fmt} library if this feature is not supported by your compiler yet):

for (int i = 1; i <= 50; ++i) {
    auto oldname = std::format("img ({}) - copy.jpg", i);
    auto newname = std::format("picture_{}.jpg", i);

And use C++17's std::fileystem::rename() to rename files without resorting to C functions:

    try {
        std::filesystem::rename(oldname, newname);
    } catch (std::filesystem::filesystem_error &err) {
        std::cerr << std::format("Error renaming file: {}\n", err.what());
    }
}

If your compiler doesn't support std::filesystem yet, you can use boost::filesystem instead.

user_54814710

I'm going to simplify what the asker had and apply some general best practices. For ease of comparison, I'm sticking to old school C++ code. Comments explaining what I did and why are embedded to keep the explanations close to what they explain.

#include <iostream>
#include <cstdio>
#include <string>
// using namespace std should only be used under controlled circumstances
// if at all.
int main()
{
    // removed all variables
    for (int i = 1; i <= 50; i++)
    {
        // Keep variable definitions close to usage unless you have a good 
        // reason not to. Can't think of any good reasons.
        std::string oldname = "img ("+ std::to_string(i) +") - copy.jpg";
        std::string newname = "picture_" + std::to_string(i) + ".jpg";
        
        // no need for the char arrays or the strcpys. Instead we will use the
        // std::string::c_str function to get the string's backing array

        // If C++17 or newer is available we could use std::filesystem::rename.
        // I'm leaving off the std:: prefix on rename because C is all we need
        if (rename(oldname.c_str(), newname.c_str()) != 0)
        { // I find that always using the optional braces prevents future bugs.
          // That's a personal opinion and choice.
            perror("Error renaming file");
        }
        else
        {
            std::cout << oldname << " renamed successfully to " << newname << std::endl;
        }
    }
    char c;
    std::cin >> std::noskipws >> c; // >> c will ignore whitespace, so the user
                                    // cannot advance simply by pressing enter.
    return 0;
}

A note on comments. If the purpose of the code cannot be inferred from the code and the variable names, consider changing the code. Only comment the stuff that's still arcane no matter how you rework it.

user_45813010
Related