Прошу оценить код

Это часть программы которая запрашивает ввод месяца.
Подразумевается, что пользователь может ввести месяц, как в числовом формате, так и слово. Для этого предусмотрен цикл проверки на ввод корректного значения.
В самом коде значение месяца будет использоваться, как для вывода на экран в строках, так и для сравнения с числами в диапазонах. Поэтому к месяцам можно обращаться либо через массив структур const strct_months months[ArSize] , либо через переменную int month = 0;.

// ввод месяца
#include <iostream>
#include <string>
#include <cctype>
#include <Windows.h>

const int ArSize = 13;

struct strct_months
{
    std::string Name;
    std::string name;
    std::string num;
};
const strct_months months[ArSize] =     // массив структур под месяца в строках и цифрах
{
    {},
    {"Январь", "январь", "1"},
    {"Февраль", "февраль", "2"},
    {"Март", "март", "3"},
    {"Aпрель", "апрель", "4"},
    {"Май", "май", "5"},
    {"Июнь", "июнь", "6"},
    {"Июль", "июль", "7"},
    {"Август", "август", "8"},
    {"Сентябрь", "сентябрь", "9"},
    {"Октябрь", "октябрь", "10"},
    {"Ноябрь", "ноябрь", "11"},
    {"Декабрь", "декабрь", "12"}
};

int main(void)
{
    SetConsoleCP(1251);
    SetConsoleOutputCP(1251);

    std::cout << "Please enter the month: ";
    std::string* month_tmp = new std::string;   // времменная переменная для проверки значения
    getline(std::cin, *month_tmp);  // ввод месяца только по русски и арабскими цифрами
    int month = 0;
    do{
    for (int i = 0; i < ArSize; i++){
        if (months[i].Name == *month_tmp)   // если значение в виде строки букв, которая начинается с заглавной
            month = i;  // присвоить порядковый номер месяца
        else if (months[i].name == *month_tmp)  // если значение в виде строки букв строчных
            month = i;  // присвоить порядковый номер месяца
        else if (months[i].num == *month_tmp)   // если значение в виде цифры
            month = i; // присвоить порядковый номер месяца
        }
        if (0 == month){        // если переменной month не присвоено значение - запрос на повторный ввод
            std::cout << "Please enter corect month: ";
            getline(std::cin, *month_tmp);
        }
    }while(!month); // делать пока month == 0;

    delete month_tmp;

    std::cout << "You entered the month " << months[month].name << std::endl;
    return 0;
}

Прошу оценить на сколько это грамотно и имеет ли это право на жизнь, если нет, то как это оптимизировать :)
P.S. В свой огород принимаю все камни :)

Предлагаю вместо if использовать switch. Упростит программу.
Вместо все время std:функция, советую написать using namespace std над структурой. Сейчас посмотрю в деле программу.

Если изучали класс, то можно ее использовать вместо структуры. Так как классы мощные в коде, и их часто используют. Ну а так не плохло

В конце советую поставить system(«pause») или _getch(), чтобы программа сразу не закрывалась. Но чтобы _getch() работало, надо подключить библиотеку conio.h

ТС, вы усложняете. Это не есть хорошо.

Зачем нужна структура, которая хранит названия месяца в разных регистрах и его номер?

Если нужно сделать «защиту» от ввода месяца в некорректном регистре, то такой метод не сработает при вводе слова «ЯНВАРЬ», например.

Номер здесь и вовсе не нужен. Во первых, потому что он нигде в коде не используется. Во вторых, потому что за него вполне сойдет индекс массива.

Почему в массиве с месяцами нулевой элемент пустой? Я бы сделал первоначальное значение индекса -1, а при нахождении введенного месяца изменял бы это значение на индекс массива.

Поскольку, у массива не может быть отрицательного индекса, мы точно знаем, что если после поиска элемента в массиве значением индекса осталось -1, то месяц не был найден.

По поводу динамического выделения памяти для буфера, то оно тут тоже ни к чему.

Вот мой вариант решения.

// ввод месяца
#include <iostream>
#include <string>

using namespace std;

int main(void)
{
    string month_list[] = {
        "январь", "февраль", "март", "апрель", "май", "июнь",
        "июль", "август", "октябрь", "ноябрь", "декабрь"
    };
    string buf;
    unsigned int moth_i = -1;

    while (moth_i == -1) {
        cout << "Please enter the month: ";
        getline(cin, buf);
        for (unsigned i = 0; i < 11; i++) {
            if (month_list[i] == buf) {
                moth_i = i;
                break;
            }
        }
        if (moth_i == -1) {
            cerr << "Invalid month name" << endl;
        }
    }

    cout << "You entered the month: " << month_list[moth_i] << endl;
    return 0;
}

По поводу регистра строк, то нормальным решением будет преобразование введенной пользователем строки к нижнему регистру и поиск уже по этому значению.

Для преобразования латинских символов, можно использовать функцию tolower из стандартной библиотеки. Или std::transform из <algorithm> (пример использования).

С кириллицей чуть посложнее, но тоже решаемо. Но это уже другая тема. В Windows, кстати. есть нестандартизированная функция strcmpi, которая сравнивает две строки без учета регистра. Попробуйте воспользоваться ей.

Nikitaz58, у тебя получилось прям по Остеру «Вредные советы» ))

(1) if — не прокатит. Здесь для каждой альтернативы своё условие.

(2) В C++ структура — это класс с членами, открытыми по умолчанию (т.е. все члены public, если явно не указано что-то другое). В остальном классы и структуры ни чем не отличаются.

(3) Г-н Страуструп советует не изобретать велосипед, а пользоваться STL. Т.е. использовать класс string вместо char[] и указателей. Возможно он ошибается... )))

Юрий, лови камни ))

(1) Фрагмент

    SetConsoleCP(1251);
    SetConsoleOutputCP(1251);

работает только если по умолчанию для консольного окна установлен юникодный шрифт. Если установлен растровый шрифт — вся красота с кириллицей отъезжает (и на ввод, и на вывод).

If the current font is a fixed-pitch Unicode font, SetConsoleOutputCP changes the mapping of the character values into the glyph set of the font, rather than loading a separate font each time it is called. This affects how extended characters (ASCII value greater than 127) are displayed in a console window. However, if the current font is a raster font, SetConsoleOutputCP does not affect how extended characters are displayed.

MSDN: SetConsoleOutputCP function

(2) Не понравилось, что у тебя в массиве months 13 «месяцев». Вдвойне не понравилось, что первый (нулевой) месяц пустой, т.е. для создания экземпляра strct_months вызывается дефолтный конструктор. И в цикле ты начинаешь проверку с 0-го элемента. В данном случае, оно прокатывает, но вообще-то так делать не надо — можно нарваться на неприятности.

(3) Не понравилось наличие строк для сравнения с номерами месяцев. Я бы использовал функцию atoi() из cstdlib для преобразования строки-номера в целое и использовал бы непосредственно это значение (с соотв. проверками).

(4) Не понравилось дублирование названий месяцев с точностью до регистра первого символа. А если пользователь введёт «МАЙ» — это считается неправильно? Или надо добавлять ещё одну альтернативу (в структуру и код)? Ну и т.д.

Обычно для таких сравнений строк проверяемую строку приводят к одному регистру, например, к нижнему. И, соответственно, шаблоны (названия месяцев, в данном случае) существуют в одном экземпляре (все символы в нижнем регистре).

Альтернативный путь: использовать для сравнения строк регулярные выражения. Для данного случая — это конечно стрельба из пушки по воробьям, в плане эффективности. Но иногда регэкспы незаменимы.

Ещё посоветовал бы («защита от дурака») перед сравнением убирать из введённой строки начальные и концевые пробельные символы. Т.е. отрабатывать ввод типа « май ».

(5) Не понравилось динамическое создание временной переменной для ввода строки (std::string* month_tmp = new std::string). Зачем такие сложности? Хватило бы и просто std::string month_tmp (соответственно далее по коду убрать оператор разыменования). У тебя тратится лишнее время на выделение памяти из кучи по new и освобождение по delete — это медленные операции. При использовании автоматической переменной код будет проще, а выполняться будет быстрее. (В данном случае, конечно, это несущественно, но надо привыкать писать аккуратный код, что бы потом не мучиться с оптимизацией.)

(6) Не понравилось, что строка может вводиться пользователем в двух местах в коде. Ввод от пользователя лучше не размазывать по коду.

(7) Я бы посоветовал код, ответственный за проверку введённой строки, оформить как функцию. Эта функция при удачном вводе месяца должна возвращать номер месяца от 0 до 11 (int), который в дальнейшем будет использован в качестве индекса для массива строк (const char *month_names[] или const std::string month_names[]), содержащего только названия месяцев в нужном регистре. Такой подход выгоден и в смысле дополнительной гибкости: название месяца может быть введено на одном языке (допустим, русском), а далее в программе будет использовано название месяца на другом языке (например, хинди), или будет использовано сокращённое название «янв», «фев» и т.д.

PS. Как всегда: объяснять дольше, чем написать свою версию кода ))

PPS. Несколько странная постановка задачи. Если бы мне, как пользователю, предложили вводить с клавиатуры данные в программу, которая понимает и названия месяцев, и их номера, то я бы конечно вводил номера, поскольку это короче и быстрее. Но с другой стороны, если ввод идёт из файла, где месяц может быть указан как строкой, так и числом, такой код имеет право на существование. Но! Тогда его надо обязательно оформлять как функцию, которая на входе получает строку, а обработку ошибок выносить в вызывающий код.

Спасибо БОЛЬШОЕ за комментарии, как всегда объективно и щедро.
Череп ты разгадал мою стратегию, я сейчас замыслил написать одну программку и реализацию разбил на несколько шагов. Эта часть и должна быть функцией на ввод и проверку введенного месяца. Хотелось, чтобы этот этап был защищен «от дурака» на 100%, но пробник не прошел... :( а казалось бы все так просто :)
Теперь подробно разберу комментарии и к следующему шагу...
TO BE CONTINUED... :)

Внимание! Это довольно старый топик, посты в него не попадут в новые, и их никто не увидит. Пишите пост, если хотите просто дополнить топик, а чтобы задать новый вопрос — начните новый.

Ответить

Вы можете использовать разметку markdown для оформления комментариев и постов. Используйте функцию предпросмотра для проверки корректности разметки.

Пожалуйста, оформляйте исходный код в соответствии с правилами разметки. Для того, чтобы вставить код в комментарий, скопируйте его в текстовое поле ниже, после чего выделите то, что скопировали и нажмите кнопку «код» в панели инструментов. Иначе ваш код может принять нечитаемый вид.

Либо производите оформление кода вручную, следующим образом:

``` #include <iostream> using namespace std; int main() { // ... } ```

Предпросмотр сообщения

Ваше сообщение пусто.