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 Bagryanova Ekaterina

Version: 1.02.0

comment:2 Changed 4 years ago by Sokolov Viacheslav

Version: 2.01.0

comment:3 Changed 4 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

Player:
В целом можно оставить как есть, но хочу акцентировать внимание вот на чем:
Используется в двух разных ипостасях (очередность игрока) и содержимое клетки. Между собой эти сущности связаны, но отличаются. NONE в качестве текущего игрока означает, что игра окончена и никто походить не может. NONE в качестве варианта хода не имеет смысла (и не должен быть допустим при вызове из интерфейса приложения).
То есть можно было бы разбить сущность на две:
CellContent??? (какой символ записан в клетке; X / O / Empty)
Player (игрок, X / O)
currentPlayer бы возращал std::optional<Player>
canMove / move принимал бы Player
Отличие между типами могло бы уберечь от ошибок - на этапе компиляции могла бы быть проверка, что программист подумал о разнице между содержимым клетки и очередностью хода и имеет в виду именно очередность хода при вызове метода. Больше кода, зато меньше возможностей ошибиться и проще модифицировать (вдруг Вы захотите добавить препятствия на игровое поле?)

28 bool can_move(Position pos) const;
30 void move(Position pos, Player player);

Если на Board лежит ответственность за текущего игрока, то в move его нет смысла передавать. Если нет, то в can_move стоит добавить (если конечно can_move - это проверка, валидно ли сделать move)

45 const int SZ = 10;
46 const int MUST = 5;

static constexpr

47 std::vector<std::vector<Player> > field;

можно std::array

StdioBoardView?:
лучше принимать не bool silent, а сделать
enum Mode{DEFAULT, SILENT, ...}, чтобы легко поддерживать новые режимы, а также избежать ошибок при смене значения флага.

13 bool split_input(std::string &input, Position &pos) const;

private?

20 const Position END_GAME = {-1, -1};
static

можно было бы из Board достать состояние поля и проверять его в тестах (это бы их упростило)

5 Board::Board() {
6 field.resize(SZ, std::vector<Player> (SZ, Player::EMPTY));
7 }

инициализацию можно произвести в списке инициализации

Board::current_player
если Board не предполагает, что ходы делаются игроками по очереди, то сам метод не вполне корректен (нет "текущего игрока")

Board::get_element
можно просто at; не хватает проверки, что позиция внутри поля

send_board - непонятное название, лучше show / print / display

26 assert(!std::cout.fail());

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

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

51 if (pos.row == END_GAME.row && pos.col == END_GAME.col) {
лучше сделать operator== для Position, можно еще пригодиться

В разные места можно было бы дописать const

comment:4 Changed 4 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed
Note: See TracTickets for help on using tickets.