Прошу оценить код
Внимание! Это довольно старый топик, посты в него не попадут в новые, и их никто не увидит. Пишите пост, если хотите просто дополнить топик, а чтобы задать новый вопрос — начните новый.
Внимание! Это довольно старый топик, посты в него не попадут в новые, и их никто не увидит. Пишите пост, если хотите просто дополнить топик, а чтобы задать новый вопрос — начните новый.
Это часть программы которая запрашивает ввод месяца.
Подразумевается, что пользователь может ввести месяц, как в числовом формате, так и слово. Для этого предусмотрен цикл проверки на ввод корректного значения.
В самом коде значение месяца будет использоваться, как для вывода на экран в строках, так и для сравнения с числами в диапазонах. Поэтому к месяцам можно обращаться либо через массив структур
const strct_months months[ArSize]
, либо через переменнуюint month = 0;
.Прошу оценить на сколько это грамотно и имеет ли это право на жизнь, если нет, то как это оптимизировать :)
P.S. В свой огород принимаю все камни :)
Предлагаю вместо if использовать switch. Упростит программу.
Вместо все время std:функция, советую написать using namespace std над структурой. Сейчас посмотрю в деле программу.
Если изучали класс, то можно ее использовать вместо структуры. Так как классы мощные в коде, и их часто используют. Ну а так не плохло
Вместо string можно использовать char[] с массивом или с указателем.
В конце советую поставить system(«pause») или _getch(), чтобы программа сразу не закрывалась. Но чтобы _getch() работало, надо подключить библиотеку conio.h
ТС, вы усложняете. Это не есть хорошо.
Зачем нужна структура, которая хранит названия месяца в разных регистрах и его номер?
Если нужно сделать «защиту» от ввода месяца в некорректном регистре, то такой метод не сработает при вводе слова «ЯНВАРЬ», например.
Номер здесь и вовсе не нужен. Во первых, потому что он нигде в коде не используется. Во вторых, потому что за него вполне сойдет индекс массива.
Почему в массиве с месяцами нулевой элемент пустой? Я бы сделал первоначальное значение индекса -1, а при нахождении введенного месяца изменял бы это значение на индекс массива.
Поскольку, у массива не может быть отрицательного индекса, мы точно знаем, что если после поиска элемента в массиве значением индекса осталось -1, то месяц не был найден.
По поводу динамического выделения памяти для буфера, то оно тут тоже ни к чему.
Вот мой вариант решения.
По поводу регистра строк, то нормальным решением будет преобразование введенной пользователем строки к нижнему регистру и поиск уже по этому значению.
Для преобразования латинских символов, можно использовать функцию tolower из стандартной библиотеки. Или
std::transform
из<algorithm>
(пример использования).С кириллицей чуть посложнее, но тоже решаемо. Но это уже другая тема. В Windows, кстати. есть нестандартизированная функция
strcmpi
, которая сравнивает две строки без учета регистра. Попробуйте воспользоваться ей.Nikitaz58, у тебя получилось прям по Остеру «Вредные советы» ))
(1) if — не прокатит. Здесь для каждой альтернативы своё условие.
(2) В C++ структура — это класс с членами, открытыми по умолчанию (т.е. все члены
public
, если явно не указано что-то другое). В остальном классы и структуры ни чем не отличаются.(3) Г-н Страуструп советует не изобретать велосипед, а пользоваться STL. Т.е. использовать класс
string
вместоchar[]
и указателей. Возможно он ошибается... )))Юрий, лови камни ))
(1) Фрагмент
работает только если по умолчанию для консольного окна установлен юникодный шрифт. Если установлен растровый шрифт — вся красота с кириллицей отъезжает (и на ввод, и на вывод).
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. Несколько странная постановка задачи. Если бы мне, как пользователю, предложили вводить с клавиатуры данные в программу, которая понимает и названия месяцев, и их номера, то я бы конечно вводил номера, поскольку это короче и быстрее. Но с другой стороны, если ввод идёт из файла, где месяц может быть указан как строкой, так и числом, такой код имеет право на существование. Но! Тогда его надо обязательно оформлять как функцию, которая на входе получает строку, а обработку ошибок выносить в вызывающий код.
Скок писал :D
Спасибо БОЛЬШОЕ за комментарии, как всегда объективно и щедро.
Череп ты разгадал мою стратегию, я сейчас замыслил написать одну программку и реализацию разбил на несколько шагов. Эта часть и должна быть функцией на ввод и проверку введенного месяца. Хотелось, чтобы этот этап был защищен «от дурака» на 100%, но пробник не прошел... :( а казалось бы все так просто :)
Теперь подробно разберу комментарии и к следующему шагу...
TO BE CONTINUED... :)