Filename sequencer with outdated C++ My path to learning C++ has been a rough one. Currently, I'm writing a project and my knowledge is very outdated. While I can write code that accomplishes the tasks required, I'd like to improve and learn new aspects of the language. 

So, I've written a class called Sequencer. It works. It's not great. It's not terrible. But it's based on old code. How would I improve the class with modern code?



#pragma once
#include &lt;regex&gt;
#include &lt;string&gt;
#include &lt;experimental/filesystem&gt;


class Sequencer
{
public:

    //////////////////////
    // CTors / DTor
    Sequencer(std::string const first_frame_in): first_frame_name(first_frame_in) 
    {
        bool result = std::regex_search(first_frame_name, first_frame_regex_results, match_pattern);
        prefix = first_frame_regex_results[1];
        std::string frame = first_frame_regex_results[2];
        ext = first_frame_regex_results[3];
        first_frame = std::stoi(frame);
        digit_padding = frame.length();
        current_frame = first_frame;
        out_prefix = "_signature";
        file_delim = ".";

        get_last_frame();
    }

    ~Sequencer() {}

    Sequencer(Sequencer const&amp;) = delete;
    void operator=(Sequencer const&amp;) = delete;


    bool get_next_frame(std::string &amp;next_frame, std::string &amp;name_out)
    {
        if (current_frame &lt;= last_frame)
        {
            tmp_num = add_padding(current_frame);
            next_frame = prefix + file_delim + tmp_num + file_delim + ext;
            name_out = prefix + out_prefix + file_delim + tmp_num + file_delim + "ppm";
            current_frame++;
            return true;
        }
        else
        {
            name_out = "";
            next_frame = "";
            return false;
        }
    }


private:

    void get_last_frame()
    {
        bool test = true;
        std::string tmpname;
        unsigned int i = current_frame + 1;
        do
        {
            tmpname = "C:\\ProgramData\\NVIDIA Corporation\\CUDA Samples\\v9.0\\3_Imaging\\MovieHasher\\data\\" + prefix + file_delim + add_padding(i) + file_delim + ext;
            test = std::experimental::filesystem::exists(tmpname);
            if (test)
                i++;
            else
                i--;
        } while (test);
        last_frame = i;
    }

    std::string add_padding(int frame_num_in)
    {
        std::string outval = std::to_string(frame_num_in);

        int length = outval.length();
        while (length &lt; digit_padding)
        {
            outval = "0" + outval;
            length = outval.length();
        }

        return outval;
    }

    std::string first_frame_name;
    std::regex match_pattern = std::regex("([a-zA-Z]*).([0-9]*).(\\w{3})");
    std::smatch first_frame_regex_results;
    std::string prefix, ext, out_prefix, tmp_num, file_delim;
    int first_frame;
    int last_frame;
    int digit_padding;
    int current_frame;

};




A sample main file to run the Sequencer class

////////////////////////
// INCLUDES

//// System
#include &lt;iostream&gt;
#include &lt;string&gt;

//// User Defined
#include "../../Sequencer.h"

int main(int argc, char **argv)
{
    std::string image_filename = "prefix.00000.ext";
    std::string out_filename = "prefix_signature.00000.ppm";
    Sequencer sequencer(image_filename);

    bool tmp = true;
    do
    {
        tmp = sequencer.get_next_frame(image_filename, out_filename);
        if (tmp)
            std::cout &lt;&lt; image_filename &lt;&lt; ", " &lt;&lt; out_filename &lt;&lt; std::endl;
    } while (tmp);

    return 0;
}
Code:


// CTors / DTor: This comment doesn't add anything that we can't see from the code.

Sometimes comments like this denote sections or regions of code in a file, but without consistently doing this for all the code, we don't know where the "constructor / destructor" section ends. I'd suggest just removing the comment.
Sequencer(std::string const first_frame_in): There's little benefit to making function arguments that are passed by value const. It doesn't affect the caller, and makes the function signature harder to read.

Perhaps this should be a const&amp; though?
When we have lots of member variables in a class, adding an m_ prefix on the names (e.g. m_current_frame) helps to distinguish them from local variables or function arguments.
first_frame_name, match_pattern, and first_frame_regex_results are all unused after the constructor, so they could just be local variables rather than members.
tmp_num and tmpname are not informative names. They should also be local variables since they are each used in only one function.
Presumably negative frame numbers can't exist. If this is the case we should be using an unsigned integer type, not a signed one.
We should check the result of the regex search and handle an error (since presumably this is going to be a user specified string).
The regex captures "zero or more" digits, but we don't check that we got a match before trying to convert the frame number. Perhaps we should require "one or more" digits / letters when matching.
Perhaps the regex pattern should use the specified file_delim too.
Using a chain of + operators for std::strings tends to create a lot of temporary strings. Using std::string::append is likely to be faster, especially if we reserve() the memory we need first.
We can add the padding digits in one operation by using std::string::insert.




Design:


I'd suggest grouping the information extracted from the first frame filename into a struct. We can then put the regex search and data extraction into a separate function, and more easily use the constructor initializer list.
Unless there's a particular reason for it, I wouldn't try finding the last frame inside the Sequencer class. It's probably simpler to restrict the "sequencer" to only generating filenames, and leave checking if the file exists for code that does actual processing.
Perhaps something like FrameFilenameGenerator might be a more accurate name than Sequencer?
We can effectively use the same code to generate the input and output filenames.
If we let the user keep track of the current frame, our generator can generate the filename for any requested frame, and doesn't have to keep track of the frame itself.




So applying the above points, we might get something like:

#include &lt;regex&gt;
#include &lt;string&gt;
#include &lt;experimental/filesystem&gt;

class FrameFilenameGenerator
{
public:

    // note: 
    // `first_frame_filename` isn't copied, so we only need a const&amp;
    // `file_delim` is a "sink argument" (i.e. we store a copy in the class), so we take the argument by value and move it into place.
    FrameFilenameGenerator(std::string const&amp; first_frame_filename, std::string file_delim = "."):
        m_file_delim(std::move(file_delim)),
        m_info(get_first_filename_info(first_frame_filename))
    {

    }

    // note: we should probably be more careful with the frame offset!
    // first_frame_number + frame_offset could overflow!
    std::string make_filename(std::uint32_t frame_offset, std::string const&amp; output_suffix = "") const
    {
        auto result_size = 
            m_info.prefix.size() + 
            output_suffix.size() + 
            m_file_delim.size() + 
            m_info.digit_padding + 
            m_file_delim.size() + 
            m_info.extension.size();

        auto result = std::string();
        result.reserve(result_size);

        result.append(m_info.prefix);
        result.append(output_suffix);
        result.append(m_file_delim);
        result.append(frame_number_to_string(m_info.first_frame_number + frame_offset, m_info.digit_padding));
        result.append(m_file_delim);
        result.append(m_info.extension);

        return result;
    }

private:

    struct FirstFilenameInfo
    {
        std::string prefix;
        std::uint32_t first_frame_number;
        std::size_t digit_padding;
        std::string extension;
    };

    static FirstFilenameInfo get_first_filename_info(std::string const&amp; first_frame_filename)
    {
        auto result = FirstFilenameInfo();

        const auto match_pattern = std::regex("([a-zA-Z]*).([0-9]*).(\\w{3})");
        std::smatch regex_results;

        if (!std::regex_search(first_frame_filename, regex_results, match_pattern))
            throw std::runtime_error("First frame filename does not meet the required format!");

        result.prefix = regex_results[1];

        if (regex_results[2].length())
        {
            result.first_frame_number = std::uint32_t{ std::stoul(regex_results[2]) };
            result.digit_padding = regex_results[2].length();
        }

        result.extension = regex_results[3];

        return result;
    }

    static std::string frame_number_to_string(std::uint32_t frame_number, std::size_t digit_padding)
    {
        auto result = std::to_string(frame_number);

        if (result.size() &gt;= digit_padding)
            return result;

        auto const zeros_needed = (digit_padding - result.size());
        result.insert(result.begin(), zeros_needed, '0');

        return result;
    }

    std::string m_file_delim;
    FirstFilenameInfo m_info;
};

#include &lt;iostream&gt;

int main()
{
    std::string image_filename = "prefix.00030.ext";
    auto generator = FrameFilenameGenerator(image_filename);

    for (auto i = 0u; i != 20u; ++i)
    {
        auto in = generator.make_filename(i);
        auto out = generator.make_filename(i, "_signature");

        //if (!std::experimental::filesystem::exists(in))
        //  break;

        std::cout &lt;&lt; in &lt;&lt; " -&gt; " &lt;&lt; out &lt;&lt; "\n";
    }

    return EXIT_SUCCESS;
}