Opened 4 years ago
Closed 4 years ago
#726 closed ожидаются исправления (задача сдана)
HW #2 (XO)
Reported by: | Bagryanova Ekaterina | Owned by: | Sokolov Viacheslav |
---|---|---|---|
Component: | HW #2 (X0) | Version: | 1.0 |
Keywords: | Cc: |
Description
Change History (4)
comment:1 Changed 4 years ago by
Version: | 1.0 → 2.0 |
---|
comment:2 Changed 4 years ago by
Version: | 2.0 → 1.0 |
---|
comment:3 Changed 4 years ago by
Type: | ожидается проверка → ожидаются исправления |
---|
comment:4 Changed 4 years ago by
Resolution: | → задача сдана |
---|---|
Status: | assigned → closed |
Note: See
TracTickets for help on using
tickets.
Player:
В целом можно оставить как есть, но хочу акцентировать внимание вот на чем:
Используется в двух разных ипостасях (очередность игрока) и содержимое клетки. Между собой эти сущности связаны, но отличаются. NONE в качестве текущего игрока означает, что игра окончена и никто походить не может. NONE в качестве варианта хода не имеет смысла (и не должен быть допустим при вызове из интерфейса приложения).
То есть можно было бы разбить сущность на две:
CellContent??? (какой символ записан в клетке; X / O / Empty)
Player (игрок, X / O)
currentPlayer бы возращал std::optional<Player>
canMove / move принимал бы Player
Отличие между типами могло бы уберечь от ошибок - на этапе компиляции могла бы быть проверка, что программист подумал о разнице между содержимым клетки и очередностью хода и имеет в виду именно очередность хода при вызове метода. Больше кода, зато меньше возможностей ошибиться и проще модифицировать (вдруг Вы захотите добавить препятствия на игровое поле?)
Если на Board лежит ответственность за текущего игрока, то в move его нет смысла передавать. Если нет, то в can_move стоит добавить (если конечно can_move - это проверка, валидно ли сделать move)
static constexpr
можно std::array
StdioBoardView?:
лучше принимать не bool silent, а сделать
enum Mode{DEFAULT, SILENT, ...}, чтобы легко поддерживать новые режимы, а также избежать ошибок при смене значения флага.
private?
20 const Position END_GAME = {-1, -1};
static
можно было бы из Board достать состояние поля и проверять его в тестах (это бы их упростило)
инициализацию можно произвести в списке инициализации
Board::current_player
если Board не предполагает, что ходы делаются игроками по очереди, то сам метод не вполне корректен (нет "текущего игрока")
Board::get_element
можно просто at; не хватает проверки, что позиция внутри поля
send_board - непонятное название, лучше show / print / display
так конечно делать не стоит (нужно ли пояснить, почему?), лучше использовать механизм исключений. Но намерение правильное.
https://en.cppreference.com/w/cpp/string/byte/isdigit
split_input - лучше именовать extractPosition; можно сделать
std::optional<Position> tryExtractPosition(const std::string& s)
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out
В разные места можно было бы дописать const