Добрый день, уважаемые форумчане!

Пролог
В далёкой-далёкой галактике проникся важностью рефакторинга.
На планете Сириус-12 на прошлом месте работы достался проект "вырви-глаз".

Очень умные ребята класса "человек-разумный" написали очень сложночитаемый код и передали нашей команде. Сами пошли вперед, на новый проект компании.
К примеру, использовалась линия отсечки длины символов 60-70, что весь код вытягивался в трубочку вниз капая сгущенкой в 1000ный номер строки.
Использовались сложные названия переменных, сущностей.
Из-за сверхсекретной мотивации в некоторых местах куча классов объявлялись в одном файле.
И т.п.

Мой руководитель, к которому я пришел, был архитектором разряда "межгалактический эксперт" по созданию телекоммуникационного оборудования, работающего 24/7 во всех открытых и заселенных системах.
С опытом С++. Также хорошо погружен в питон, си-шарп. И, наверное, во много чего еще.

Завязка
Он уделял много внимания простоте кода. Чтобы код становился всё более читаемым.
Так, за много итераций рефакторинга в процессе работы с проектом, получалось лучше вникать в очередном модифицируемый модуль — рефакторить и улучшать читаемость.

Попутно работая с питоном я проникался идеями созданием читаемого кода. Так, чтобы { не мешала читать — правее её за объявление, куда уже никто не смотрит.
Что если можно поставить auto — ставить auto.
Что если нужен простой класс таймер — сделать простой класс таймер, а не супер-шаблоидный класс с кучами кейсов и сверхфункциональным api.

Развитие действия
И вот на следующем месте работы оказалось нужно улучшить конфиг, чтение из него и добавление функционала далее.

void Config::load(xml_wrap::NodeConst const & root_node)
{
    // Call base class first.
    ipc::SimpleConfig::load(root_node("ipc"));

    auto const micex_node = root_node("micex");
    {
        auto const connection_node = micex_node("connection");
        m_asts_connection_attempts       = connection_node.attribute<unsigned int>("attempts", "6");
        m_asts_connection_attempt_period = std::chrono::seconds(connection_node.attribute<unsigned int>("attempt-period", "300"));

        std::string asts_connection_params;
        auto const param_nodes = connection_node.children("param");
        for(auto const & param_node : param_nodes)
        {
            asts_connection_params += param_node.attribute<std::string>("name");
            asts_connection_params += '=';
            asts_connection_params += param_node.attribute<std::string>("value");
            asts_connection_params += "\r\n";
        }
        m_asts_connection_params = std::move(asts_connection_params);
    }
    {
        std::vector<subscription_t>                     asts_subscriptions;
        std::unordered_map<std::string, std::size_t>    asts_subscriptions_index;
        auto const table_nodes = micex_node("tables").children("table");
        for(auto const & table_node : table_nodes)
        {
            subscription_t subscription; // Таблица_имя, режим вычитки, ...
            subscription.m_table = table_node.attribute<std::string>("micex-name");
            if(!asts_subscriptions_index.emplace(subscription.m_table, asts_subscriptions.size()).second)
                // TODO: duplicated table warning.
                continue;
            subscription.m_snapshot = table_node.attribute<bool>("load-complete" , "true" );
            subscription.m_updates  = table_node.attribute<bool>("enable-refresh", "false");
            subscription.m_publish  = table_node.attribute<bool>("enable-publish", "true" );
            asts_subscriptions.push_back(std::move(subscription));
        }
        m_asts_subscriptions       = std::move(asts_subscriptions      );
        m_asts_subscriptions_index = std::move(asts_subscriptions_index);
    }

    ...
}

Я добавляю if после строчки:

            asts_subscriptions.push_back(std::move(subscription));

            if(table_node.hasAttribute("extra-load-complete-at") && !subscription.m_updates)
            {
                fill_second_snapshots_subscriptions( subscription, table_node.attribute<std::string>( "extra-load-complete-at" ) );
            }
 }
 m_asts_subscriptions       = std::move(asts_subscriptions      );
 m_asts_subscriptions_index = std::move(asts_subscriptions_index);

Сколько-то в run-time пытаюсь понять, почему не получается. Оказалось, был move, который я не заметил.
Вопрос — зачем?
Это обычный конфиг. Здесь не нужно ничего такого экстра-пупер.
Пиши себе обычный понятный код.

Далее, я хотел if без скобок написать. Но нет. По ревью оказалось нельзя.
Вопрос — зачем? Кто-то когда-то написал что скобки нужны. Но тот же clion сразу делает отступ, чтобы случайно не было путаницы.
А получаем нагрузку в паре лишних символов — {}.
Когда смотрим на файл с одной функцией — может и ок. Но если что-то увесистей, то доп символами нагружаем зрительный канал, мозг. Лишним.
Т.е. мы можем получить код, который:
1) Быстро читается
2) Понятный
Но вместо этого пользуемся другой мотивацией.

Сейчас вообще скажу страшное. Архитектор в cpp делал:

using namespace std;

И можно было писать просто:

string str =...
ifstream ifstr =...
whatever

без "std::" каждый раз, Карл!

Я был возмущен! Ведь из каждого угла всех квадратных строений всех известных обитаемых галактик говорится:
"Это bad practice! Never, never do it."

А потом я офигел от того, что из-за отсутствия 3ех букв и двух двоеточий код распрямился, дышал полной грудью и я с ним становился на ты.

Кульминация
Делаю далее.
Стараюсь не запутаться с преобразованием времени.
Пишу многословно, чтобы было понятней прочитать на следующей неделе.
Стараюсь сделать auto(сейчас понял, что сделал не везде):

void Config::fill_second_snapshots_subscriptions(subscription_t & subscription, const std::string & time_from_config)
{
    subscription.m_snapshot = true;

    std::regex input_mask("^[0-2][0-9]:[0-5][0-9]:[0-5][0-9]$");

    if(! regex_match(time_from_config, input_mask))
    {
        std::ostringstream oss_err;
        oss_err << "Invalid time format for a second snapshot "
                << time_from_config
                << " for table "
                << subscription.m_table
                << ". Valid is HH:MM:SS";
        throw std::logic_error(oss_err.str());
    }

    std::time_t raw_time = std::time(nullptr);
    std::tm     current_time_stamp = *std::localtime(&raw_time);

    std::istringstream ss(time_from_config);
    ss >> std::get_time(&current_time_stamp, "%H:%M:%S");

    auto time_stamp_with_time_from_config = std::mktime(&current_time_stamp);

    auto time_to_start_snapshot = system_clock::from_time_t(time_stamp_with_time_from_config);
    auto now = system_clock::now();

    if(time_to_start_snapshot > now )
    {
        subscriptions_second_snapshots[subscription] = time_to_start_snapshot;
    }
}

И получаю на ревью:

const auto& time_stamp_with_time_from_config = std::mktime(&current_time_stamp);
const auto& time_to_start_snapshot = system_clock::from_time_t(time_stamp_with_time_from_config);
const auto& now = system_clock::now();

Т.к. RVO, Карл!
Why?
Why здесь?

Развязка
"Чтобы не терять навык."

Эпилог
Вспоминаю выступление Скотта Мейерса.
Он говорил(на память), что "Есть фича. И есть варианты/контекст её использования". Есть граничные условия.
Что хорошо бы использовать фичу, где её можно использовать надежно, где она к месту.
Это я, наверное, про начальный move, который ни к земному селу, ни к марсианскому городу. Который просто усложнил чтение.

https://youtu.be/9vyh1APsMAE?t=2258


Ок. Есть RVO. Но в конфиге зачем его юзать?

Добрый день, уважаемые форумчане!

Пролог
В далёкой-далёкой галактике проникся важностью рефакторинга.
На планете Сириус-12 на прошлом месте работы достался проект "вырви-глаз".

Очень умные ребята класса "человек-разумный" написали очень сложночитаемый код и передали нашей команде. Сами пошли вперед, на новый проект компании.
К примеру, использовалась линия отсечки длины символов 60-70, что весь код вытягивался в трубочку вниз капая сгущенкой в 1000ный номер строки.
Использовались сложные названия переменных, сущностей.
Из-за сверхсекретной мотивации в некоторых местах куча классов объявлялись в одном файле.
И т.п.

Мой руководитель, к которому я пришел, был архитектором разряда "межгалактический эксперт" по созданию телекоммуникационного оборудования, работающего 24/7 во всех открытых и заселенных системах.
С опытом С++. Также хорошо погружен в питон, си-шарп. И, наверное, во много чего еще.

Завязка
Он уделял много внимания простоте кода. Чтобы код становился всё более читаемым.
Так, за много итераций рефакторинга в процессе работы с проектом, получалось лучше вникать в очередном модифицируемый модуль — рефакторить и улучшать читаемость.

Попутно работая с питоном я проникался идеями созданием читаемого кода. Так, чтобы { не мешала читать — правее её за объявление, куда уже никто не смотрит.
Что если можно поставить auto — ставить auto.
Что если нужен простой класс таймер — сделать простой класс таймер, а не супер-шаблоидный класс с кучами кейсов и сверхфункциональным api.

Развитие действия
И вот на следующем месте работы оказалось нужно улучшить конфиг, чтение из него и добавление функционала далее.

void Config::load(xml_wrap::NodeConst const & root_node)
{
    // Call base class first.
    ipc::SimpleConfig::load(root_node("ipc"));

    auto const micex_node = root_node("micex");
    {
        auto const connection_node = micex_node("connection");
        m_asts_connection_attempts       = connection_node.attribute<unsigned int>("attempts", "6");
        m_asts_connection_attempt_period = std::chrono::seconds(connection_node.attribute<unsigned int>("attempt-period", "300"));

        std::string asts_connection_params;
        auto const param_nodes = connection_node.children("param");
        for(auto const & param_node : param_nodes)
        {
            asts_connection_params += param_node.attribute<std::string>("name");
            asts_connection_params += '=';
            asts_connection_params += param_node.attribute<std::string>("value");
            asts_connection_params += "\r\n";
        }
        m_asts_connection_params = std::move(asts_connection_params);
    }
    {
        std::vector<subscription_t>                     asts_subscriptions;
        std::unordered_map<std::string, std::size_t>    asts_subscriptions_index;
        auto const table_nodes = micex_node("tables").children("table");
        for(auto const & table_node : table_nodes)
        {
            subscription_t subscription; // Таблица_имя, режим вычитки, ...
            subscription.m_table = table_node.attribute<std::string>("micex-name");
            if(!asts_subscriptions_index.emplace(subscription.m_table, asts_subscriptions.size()).second)
                // TODO: duplicated table warning.
                continue;
            subscription.m_snapshot = table_node.attribute<bool>("load-complete" , "true" );
            subscription.m_updates  = table_node.attribute<bool>("enable-refresh", "false");
            subscription.m_publish  = table_node.attribute<bool>("enable-publish", "true" );
            asts_subscriptions.push_back(std::move(subscription));
        }
        m_asts_subscriptions       = std::move(asts_subscriptions      );
        m_asts_subscriptions_index = std::move(asts_subscriptions_index);
    }

    ...
}

Я добавляю if после строчки:

            asts_subscriptions.push_back(std::move(subscription));

            if(table_node.hasAttribute("extra-load-complete-at") && !subscription.m_updates)
            {
                fill_second_snapshots_subscriptions( subscription, table_node.attribute<std::string>( "extra-load-complete-at" ) );
            }
 }
 m_asts_subscriptions       = std::move(asts_subscriptions      );
 m_asts_subscriptions_index = std::move(asts_subscriptions_index);

Сколько-то в run-time пытаюсь понять, почему не получается. Оказалось, был move, который я не заметил.
Вопрос — зачем?
Это обычный конфиг. Здесь не нужно ничего такого экстра-пупер.
Пиши себе обычный понятный код.

Далее, я хотел if без скобок написать. Но нет. По ревью оказалось нельзя.
Вопрос — зачем? Кто-то когда-то написал что скобки нужны. Но тот же clion сразу делает отступ, чтобы случайно не было путаницы.
А получаем нагрузку в паре лишних символов — {}.
Когда смотрим на файл с одной функцией — может и ок. Но если что-то увесистей, то доп символами нагружаем зрительный канал, мозг. Лишним.
Т.е. мы можем получить код, который:
1) Быстро читается
2) Понятный
Но вместо этого пользуемся другой мотивацией.

Сейчас вообще скажу страшное. Архитектор в cpp делал:

using namespace std;

И можно было писать просто:

string str =...
ifstream ifstr =...
whatever

без "std::" каждый раз, Карл!

Я был возмущен! Ведь из каждого угла всех квадратных строений всех известных обитаемых галактик говорится:
"Это bad practice! Never, never do it."

А потом я офигел от того, что из-за отсутствия 3ех букв и двух двоеточий код распрямился, дышал полной грудью и я с ним становился на ты.

А ешё, оказывается, в hpp можно не писать название аргумента, если понятно из типа о чём идет речь:

void fill_second_snapshots_subscriptions(subscription_t &, ...);

Кульминация
Делаю далее.
Стараюсь не запутаться с преобразованием времени.
Пишу многословно, чтобы было понятней прочитать на следующей неделе.
Стараюсь сделать auto(сейчас понял, что сделал не везде):

void Config::fill_second_snapshots_subscriptions(subscription_t & subscription, const std::string & time_from_config)
{
    subscription.m_snapshot = true;

    std::regex input_mask("^[0-2][0-9]:[0-5][0-9]:[0-5][0-9]$");

    if(! regex_match(time_from_config, input_mask))
    {
        std::ostringstream oss_err;
        oss_err << "Invalid time format for a second snapshot "
                << time_from_config
                << " for table "
                << subscription.m_table
                << ". Valid is HH:MM:SS";
        throw std::logic_error(oss_err.str());
    }

    std::time_t raw_time = std::time(nullptr);
    std::tm     current_time_stamp = *std::localtime(&raw_time);

    std::istringstream ss(time_from_config);
    ss >> std::get_time(&current_time_stamp, "%H:%M:%S");

    auto time_stamp_with_time_from_config = std::mktime(&current_time_stamp);

    auto time_to_start_snapshot = system_clock::from_time_t(time_stamp_with_time_from_config);
    auto now = system_clock::now();

    if(time_to_start_snapshot > now )
    {
        subscriptions_second_snapshots[subscription] = time_to_start_snapshot;
    }
}

И получаю на ревью:

const auto& time_stamp_with_time_from_config = std::mktime(&current_time_stamp);
const auto& time_to_start_snapshot = system_clock::from_time_t(time_stamp_with_time_from_config);
const auto& now = system_clock::now();

Т.к. RVO, Карл!
Why?
Why здесь?

Развязка
"Чтобы не терять навык."

Эпилог
Вспоминаю выступление Скотта Мейерса.
Он говорил(на память), что "Есть фича. И есть варианты/контекст её использования". Есть граничные условия.
Что хорошо бы использовать фичу, где её можно использовать надежно, где она к месту.
Это я, наверное, про начальный move, который ни к земному селу, ни к марсианскому городу. Который просто усложнил чтение.

https://youtu.be/9vyh1APsMAE?t=2258


Ок. Есть RVO. Но в конфиге зачем его юзать?