Opened 4 years ago

Closed 4 years ago

#713 closed ожидается проверка (задача сдана)

HW #2

Reported by: samoylov.viktor Owned by: Sokolov Viacheslav
Component: HW #2 (X0) Version: 2.0
Keywords: Cc:

Description

Здравствуйте, вчера вечером закоммитил(дату коммита вроде посмотреть можно), но забыл создать тикет, простите пожалуйста.

Change History (3)

comment:1 Changed 4 years ago by Sokolov Viacheslav

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

В условии Bad move

В can_move не хватает проверки is_inside_field

Возможно, удобнее во всех перечислениях иметь ZERO и CROSS в одном и том же порядке

у методов не хватает константности

В качестве массивов безопаснее использовать std::array (преимущества описаны на cppreference)

Предлагаю переименовать класс Pointer, потому что из названия непонятно, зачем он нужен, а pointer в языке уже что-то конкретное значит.

8 assert(printf("FAILED in %s %s line %zu\n", filename, func, lineNum) > 0);

в release-сборке печать сообщения будет вырезана. По этой причине внутри assert не стоит располагать ничего с side-эффектами.

В тестах вместо 123456 можно было бы давать словесное описание содержимого (например, test_get_state3 --> test_x_wins_diagonal, test_get_state6 --> test_full_board_draw)

По условию, если ход невалидный - UB. Тем самым в тестах - UB. Нужно либо поправить гарантии класса (и прописать их в заголовочном файле), либо поправить UB в тестах.

get_current_player: текущего игрока может и не быть, если игра окончена. Предлагаю по этой причине в интерфейсе использовать std::optional и добавить тест на nullopt

Board::change_current_player:
лучше сделать свободную функцию, которая по Player вернет другого игрока. (лучше, потому что не в терминах класса)

Стоит добавить проверки инвариантов, например, в is_empty должно выполняться is_inside_field, в move - can_move и тд

39 field[y][x] = (Cell)player;
небезопасное преобразование, закладывается на значения перечислений. Лучше сделать свободную constexpr функцию (в которой в принципе допустимо сделать такую же реализацию!), которую можно протестировать и легко менять

непонятно, какая мотивация использовать this-> (где-то используется, где-то нет)

is_inside_field(i, j) && field[j][i]
если сделать operator()(int x, int y), то запись field(i, j) будет выглядеть так же и труднее будет ошибиться с порядком аргументов

Набор направлений стоит вынести в массив и в is_winner итерироваться по этому массиву

Непонятно, почему Mode - это свойство Controller. Кажется, это свойство View.

лучше переименовать read_move -> read_valid_move или как-то так (тогда возвращаемое значение уместнее optional)

В main дублируется код.

Ввод 0-0 обрабатывается неправильно.

comment:2 Changed 4 years ago by samoylov.viktor

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

Исправил и добавил всё, кроме operator(), ибо не думаю, что он сильно нужен

comment:3 Changed 4 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed

ход 0 0 неверно определяется, как невалидный

34 std::vector<std::pair<int, int>> directions = {
35 {1, 0}, {0, 1}, {1, 1}, {1, -1}
36 };

static const

4 #include "StdioBoardView?.h"
5 #include "NcursesBoardView?.h"
6 #include <ncurses.h>

не нужны в include/Controller.h
там достаточно просто forward declaration на View и Board

Note: See TracTickets for help on using tickets.