solve homework - #6
Conversation
| std::vector<std::string> SplitString(const std::string& data) { | ||
| return {}; | ||
| } | ||
| #include <string> |
There was a problem hiding this comment.
В первую строку принято помещать включение одноимённого хидера #include "utils.hpp". Остальные инклюды идут после и отделяются от одноимённого хидела пустой строкой
| } | ||
| #include <string> | ||
| #include <string_view> | ||
| #include <vector> |
There was a problem hiding this comment.
Между инклюдами и первой строчкой кода принято размещать пустую строку (так у текста в файле появляется структура)
| std::vector<std::string> SplitString(std::string const& data) { | ||
| std::vector<std::string> finish_array; | ||
| std::string active_data = ""; | ||
| bool brackets_condition = false; |
There was a problem hiding this comment.
Вас можно похвалить за выбор говорящих имён для переменных. Но над именами можно ещё подумать. Например, final_array лечше, чем finish_array, а ещё лучше result или res. active_data я бы переназвал как word или token
| std::vector<std::string> finish_array; | ||
| std::string active_data = ""; | ||
| bool brackets_condition = false; | ||
| for (char const& active_element : data) { |
There was a problem hiding this comment.
Вместо имени active_element можно просто ch или даже c. Если хотите избегать коротких имён, то больше подходит имя current_char
А ещё значение элементарного типа тут лучше просто копировать, чем брать по ссылке: for (const char active_element : data) {
| if (active_element == '(') brackets_condition = true; | ||
| } | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Прошу убрать ненужные ; после закрывающих блок скобок
|
|
||
| for (char c : data) { | ||
| if ((c == '0') || (c == '1') || (c == '2') || (c == '3') || (c == '4') || | ||
| (c == '5') || (c == '6') || (c == '7') || (c == '8') || (c == '9')) { |
There was a problem hiding this comment.
Лучше так: if (c >= '0' && c <= '9') {
| } else { | ||
| int_number = atoi(number.c_str()); | ||
| int_numbers[n] = int_number; | ||
| n = n + 1; |
There was a problem hiding this comment.
Вместо трёх строк 20-22 я бы написал numbers.push_back(std::stoi(number_part)). И переменные int_number и n больше не нужны
| n = n + 1; | ||
| number.clear(); | ||
| if ((c == '+') || (c == '-') || (c == '*') || (c == '/')) { | ||
| action = c; |
There was a problem hiding this comment.
Кстати, на будущее замечу, что если во входной строке не окажется арифметического оператора, то переменная action останется вообще неинициализорованной, и обращение к ней приведёт к undefined behavior. Лучше в коде такого не допускать
| } | ||
| } | ||
|
|
||
| int_numbers[n] = atoi(number.c_str()); |
There was a problem hiding this comment.
numbers.push_back(std::stoi(number_part));
| } | ||
| if (action == '/') { | ||
| answer = (int_numbers[0] / int_numbers[1]); | ||
| } |
There was a problem hiding this comment.
Вместо сцепленных if для целочисленных типов рекомендуется использовать оператор switch (но это вопрос вкуса/кодстайла)
2 participants
No description provided.