HW #2 (XO)

[Sokolov Viacheslav]

6 / Проверить ход на корректность, если бы он был следующим. */

комментарий куда-то не туда

27 FieldState? game_field[10][10];
Делать поля классов публичными плохо, потому что в таком случае кто угодно снаружи может сломать внутренние инварианты

3 #define DO_CHECK(EXPR) Test::check(EXPR, func, FILE, LINE);

неудачное расположение макроса: могут быть другие тесты, им тоже понадобится этот макрос

BoardTest? стоит пометить как final

12 Board & cur_board;
13 bool exit = false;

та же история, поля не стоит делать публичными

Кажется, лучше все-таки оставить BoardView? без полей и конструктора (выигрыш от них незначительный), тогда это будет так называемый чистый интерфейс

5 #define X Board::Player::X
6 #define O Board::Player::O

в современном C++ нет потребности в подобных минах замедленного действия (попробуйте, например, поменять порядок макросов O и DO_CHECK)
можно написать
static constexpr const auto X = Board::Player::X; (можно еще и inline)

тесты стоит назвать более осмысленно, чем 1,2,3,4,5 - давайте названия исходя из того, что именно функция делает (например, test_move_out_of_field)

хорошо бы еще проверить, что бывает ничья

13 return (check_row(x, y, 0, 1) 14 check_row(x, y, 1, 0) 15 check_row(x, y, 1, 1)

16 check_row(x, y, 1, -1));

удобнее завести набор направлений directions {{0, 1}, {1, 0}, {1, 1}, {1, -1}}

из названия count_in_a_row неясно, включается ли исходная клетка

размеры поля и количество знаков до победы стоит вынести в именованные константы

70 if (x < 0

y < 0

x >= 10

y >= 10) {

71 return false;
72 }

проверку стоит вынести в отдельную функцию, она используется в нескольких местах

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

сейчас некорректно обработается ввод
" 0-0"

GameController? и Board можно выделять на стэке в main

Сейчас между view дублируется логика:

91 if (cur_board.can_move(cur_y, cur_x, cur_player)) {
92 cur_board.move(cur_y, cur_x, cur_player);
93 correct_read = true;
94 }

89 if (!exit) {
90 cur_board.move(x, y, cur_player);
91 }

лучше вынести ее в controller, а на view оставить только ответственность за ввод / вывод

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

81 if (x == y && y == -1) {
82 exit = true;
83 }

так плохо, потому что происходит нетривиальное связывание каких-то значений с каким-то конкретным смыслом, что не закреплено явно через отдельный метод / именованную константу / ...

[Sokolov Viacheslav]

не хватает const спецификаторов (например, can_move, get_state, ... явно не должны менять состояние класса)

30 std::vector < std::pair <int, int> > directions = {{0, 1}, {1, 0}, {1, 1}, {1, -1}};
static const

31 const static int size_ = 10;
32 const int number_in_a_row = 5;

лучше constexpr

33 FieldState? game_field[size_][size_];

лучше std::array (будет проверка выхода за границы)

7 enum GameMode?{
8 DEFAULT,
9 SILENT,

10 NCURSE
11 };

стоит вынести наружу, потому что иначе базовый класс BoardView? знает про всех своих наследников

24 BoardView::Point read_input() override;
можно просто Point?

StdioBoardView? стоит вынести в отдельный файл

3 #include "Board.h"

на самом деле не нужен в include/BoardView.h, достаточно forward declaration
(аналогично с Controller)

4 #include <ncurses.h>

не нужен в include/NcursesBoardView.h

20 bool final = false;
лучше именовать как-то иначе, потому что final - выделенное слово в c++ (например, влияет на подсветку)

1 #include <iostream>
не нужен в Board.cpp?

31 for (int i = 0; i < 4; i++) {
32 win |= (check_row(x, y, directions[i].first, directions[i].second));
33 }

можно
for (const auto[dx, dy] : directions)

80 if (number_of_x + number_of_o == size_ * size_) {
81 return GameState::draw;
82 }
83 return (number_of_o > number_of_x) ? GameState::turn_x : GameState::turn_o;
можно добавить assert-ы (есть несколько инвариантов про количество x и o на поле)

23 if (point.x_ == point.y_ && point.y_ == -1)

выделенное значение лучше явно обозначить статической константой - будет проще его согласовано менять

cur_board.get_state() достаточно дорогая операция, лучше посчитать один раз

отсутствует проверка failbit/badbit у cin/cout

[Sokolov Viacheslav]

typo
20 !(point.x_ == exit_code.x_ && point.y_ == exit_code.x_)) {
24 if (point.x_ == exit_code.x_ && point.y_ == exit_code.x_) {
по этой причине лучше делать operator ==, !=.

в проекте остался src/BoardView.cpp

cur_board.get_state() достаточно дорогая операция, лучше посчитать один раз

осталось актуально