Opened 4 years ago

Closed 4 years ago

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

HW #2

Reported by: Ruslan Salkaev Owned by: Sokolov Viacheslav
Component: HW #2 (X0) Version: 2.0
Keywords: Cc:

Description

Доп сделаю к следующей попытке

Change History (5)

comment:1 Changed 4 years ago by Ruslan Salkaev

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: ожидается проверкаожидаются исправления

Неправильно работает на данных из условия.

Makefile:

3 INC = -Iinclude -lncurses

лучше разбивать команды этапа компиляции и компоновки. Из названия можно подумать, что речь про инклуды, но нет

не уверен насчет Player::None, поскольку не во всех контекстах это валидное значение перечисления. Предлагаю подумать над использованием std::optional вместо этого.

FieldState? не очень удачное название, потому что речь на самом деле не про все поле, а про одну клетку

Вместо count_* лучше сделать один метод count, который принимает направление из набора {{1,0}, {0,1}, {1,-1}, {1,1}}

Про readCoordinates(Point &p)
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out
Лучше сделать Point readCoordinates()

printResult(Point p)
непонятно, какой смысл вкладывается в Point в этой и аналогичных функциях, почему именно такой интерфейс

Непонятно, зачем в BoardView? поле

Board *board_;

7 const int BOARD_START_X = 2;
9 const int BOARD_START_Y = 2;

11 const int STEP_CODE = -2;

непонятно, зачем эти константы в заголовочном файле; в C++ константы лучше делать static constexpr

24 Point p;
25 p.x = 0, p.y = 0;
26 b.move(p, Player::O);

можно короче:
b.move({0,0}, Player::O)

testDraw:
можно оформить двумя вложенными циклами, будет короче

Конструктор Board:
все можно вынести в список инициализации; цикл

6 for (int i = 0; i < BOARD_SIZE; ++i) {
7 for (int j = 0; j < BOARD_SIZE; ++j) {
8 field_[i][j] = FieldState::None;
9 }

10 }

ничего не делает

Board::move:
стоит добавить assert(turns_left_ > 0)

149 assert(0 <= x && x < BOARD_SIZE);
150 assert(0 <= y && y < BOARD_SIZE);
стоит завести функцию inside(int x, int y) и использовать ее (нужна больше одного раза)

Controller.cpp

3 #include "StdioBoardView?.h"
4 #include "NcursesBoardView?.h"

не нужны

11 Point p;
12 p.x = 0, p.y = 0;

-> Point p{0,0};

6 assert(argc == 1
argc == 2);

лучше обработать в штатном режиме

30 if (point == FieldState::None) {
31 std::cout << '.';
32 continue;
33 }
34 if (point == FieldState::X) {
35 std::cout << 'X';
36 continue;
37 }
38 if (point == FieldState::O) {
39 std::cout << 'O';
40 continue;
41 }

лучше switch, потому что он позволяет проверить, что все значения перечисления разобраны

Отсутствует проверка успешности ввода / вывода

comment:4 Changed 4 years ago by Ruslan Salkaev

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

Optional добавить не получилось, остальное попробавал исправить

comment:5 Changed 4 years ago by Sokolov Viacheslav

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

После * move: просят еще пробел вывести

12 virtual bool nextMove(Point p, Player player, Board &board) const = 0;
13
14 virtual void drawField(Player player, Board &board) const = 0;

const Board& ?

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

в silent режиме не выводится поле в конце игры

ход 0 0a считается корректным

 54     if (count(p, check_state, 0, 1) >= GOAL_LENGTH) {
 55         setWinner(check_state);
 56         return;
 57     }
 58     if (count(p, check_state, 1, 0) >= GOAL_LENGTH) {
 59         setWinner(check_state);
 60         return;
 61     }
 62     if (count(p, check_state, 1, 1) >= GOAL_LENGTH) {
 63         setWinner(check_state);
 64         return;
 65     }
 66     if (count(p, check_state, 1, -1) >= GOAL_LENGTH) {
 67         setWinner(check_state);
 68         return;
 69     }

можно было бы вынести перебор позиций в цикл по {{1,0}, {0,1}, {1,-1}, {1,1}}

17 if (player == Player::X) {
18 player = Player::O;
19 } else if (player == Player::O) {
20 player = Player::X;
21 }

стоило бы вынести в отдельную функцию

21 if(std::cout.fail())
22 std::abort();

было бы лучше организовать через исключения

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

 54         if (player == Player::X)
 55             std::cout << "X move: ";
 56         if (player == Player::O)
 57             std::cout << "O move: ";
Note: See TracTickets for help on using tickets.