Feature/adding homework - #5
Conversation
ilya-kadochnikov
left a comment
ilya-kadochnikov
left a comment
There was a problem hiding this comment.
Как я понял, вместо task2, вы решили выполнить более сложное задание, а именно написать полноценный парсер арифметических выражений, учитывающей скобочки и приоритет операторов. То что у вас получилось - уже хорошо
| /*for (const auto& word : SplitString("asdas das das fgag (adasd 1fas)")) { | ||
| std::cout << word << "\n"; | ||
| } | ||
| */ |
There was a problem hiding this comment.
В финальном коде не должно быть закомментированных кусков
|
|
||
| int Calculate(const std::string& data) { | ||
| return 0; | ||
| auto split_string(const std::string &str) { |
There was a problem hiding this comment.
Если мы придерживаемся кодстайлу Google C++, то имена функций должны быть PascalCase, т. е. SplitString, а не split_string
| return res; | ||
| } | ||
|
|
||
| std::string prep_string(std::string str) { |
There was a problem hiding this comment.
PrepareString(const std::string& str) - нет смысла сокращать слова, и строку здесь лучше передать по константной ссылке
Кроме того, если эта функция не используется, смело удаляйте её из финальной версии. Если она попала в систему контроля версий, её всегда можно будет восстановить
| f_str += str[i]; | ||
| } | ||
| return f_str; | ||
| } |
There was a problem hiding this comment.
Можно короче:
std::string BraceClipper(const std::string& str) {
return str.substr(1, str.size() - 2);
}
| } | ||
|
|
||
| float conductor(std::vector<float> values, std::vector<char> flags, | ||
| int num_of_operations) { |
There was a problem hiding this comment.
Не понял, почему такое имя выбрано для функции
| values.insert(values.begin() + i - 1, temp_value); | ||
|
|
||
| flags.erase(flags.begin() + i - 1, flags.begin() + i + 2); | ||
| flags.insert(flags.begin() + i - 1, '~'); |
There was a problem hiding this comment.
А вот тут у вас баг))) Вы в цикле for пробегаете по массиву, на лету добавляя/удаляя элементы из середины массива. Это всегда чревато неприятными багами. Чтобы убедиться в наличии бага, попробуйте распарсить строку "8 / 2 / 2 / 2". Быстрый и грязный фикс - вставить после flags.insert(flags.begin() + i - 1, '~'); следующую строку:
i -= 2;
|
|
||
| flags.erase(flags.begin() + i - 1, flags.begin() + i + 2); | ||
| flags.insert(flags.begin() + i - 1, '~'); | ||
|
|
There was a problem hiding this comment.
Тот же баг. Проверка: "3 - 1 - 1 - 1". Фикс: i -= 2;
| } | ||
| } | ||
| } | ||
| for (int i = 0; i < 3; ++i) { |
There was a problem hiding this comment.
Вот этот цикл неоправданно сложный. Вам всего-то осталось проверить, есть ли на поле свободная клетка
for (int i = 0; i < 3; ++i) {
for (int j = 0; j < 3; ++j) {
if (m_grid[i][j] == Field::Free)
return GameState::Running;
}
}
return GameState::Draw;
// конец метода
| return result; | ||
| } | ||
|
|
||
| std::string GStoS(GameState f) { |
There was a problem hiding this comment.
GStoS - плохое имя. Но, признаюсь, что на занятии я сам породил чудовищное имя NandC как сокращение для NoughtsAndCrosses (просто на доске тяжело мелом писать, и вечно места не хватает). Прошу плохому примеру не следовать и слова не сокращать.
| return "Noughts win"; | ||
| } else if (f == GameState::C_wins) { | ||
| return "Crosses win"; | ||
| } |
There was a problem hiding this comment.
Тут больше switch подходит, или вообще можно воспользоваться ассоциированным массивом
std::map<GameState, std::string> game_state_as_string = {
{GameState::Running, "Running"},
{GameState::Draw, "Draw"},
{GameState::N_wins, "Nought win"},
{GameState::C_wins, "Crosses win"},
};
2 participants
My homework