HW #2

[Sokolov Viacheslav]

Целевая платформа - linux. На ней не собирается. "w" в "-lncursesw", видимо, означает Windows?

make all не собирает все возможные исполняемые файлы

Статические константы класса можно смело именовать КАПСОМ, вряд ли это приведет к какому-то конфликту. Статические константы во многом отличаются от членов, поэтому именовать их разным образом вполне оправдано.

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

Про Sign
В целом можно оставить как есть, но хочу акцентировать внимание вот на чем:
Используется в двух разных ипостасях (очередность игрока) и содержимое клетки. Между собой эти сущности связаны, но отличаются. Empty в качестве текущего игрока означает, что игра окончена и никто походить не может. Empty в качестве варианта хода не имеет смысла (и не должен быть допустим при вызове из интерфейса приложения).
То есть можно было бы разбить сущность на две:

• CellContent? (какой символ записан в клетке; X / O / Empty)
• Player (игрок, XPlayer / OPlayer)

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

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

BoardView?:
можно сделать иначе, считать, что этот класс связан с одним конкретным экземпляром Board (при конструировании) и убрать Board из методов.

Про int &x, int &y:
​http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out
лучше всего сделать
struct Position{int x; int y;};
использовать его везде, где используются x и y
и BoardView? сделать метод (например)
virtual std::optional<Position> retrieveNextPosition() = 0;
optional или нет зависит от того, как хочется устроить обработку ситуации "разрыв соединения" / предварительное окончание игры / ...

Какая мотивация именовать printField , а не более прямолинейно printBoard?

Для игр есть более специализированный термин Outcome, поэтому лучше именовать не
printResult, а printOutcome.

В testMove* стоит добавить перед каждым вызовом move проверку на возможность его выполнения - вдруг с точки зрения модели ход невалиден (а выполнение невалидного хода - UB, в частности, может быть, внутри класса уже подожжен фитиль, который взорвется, но не сразу)

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

Board::currentPlayer

8 if (_cntO == _cntX)
9 return O;

10 if (_cntX + 1 == _cntO)
11 return X;
12 assert(false && "can't get current player");

этот инвариант должен всегда выполняться, поэтому стоит вынести в отдельный метод проверку счетчиков, в этом месте поставить assert, что инвариант соблюдается, тогда можно будет оформить как if..else

Стоит завести массив Directions, в котором перечислены возможные направления

isInRowOneSign:
стоит поставить assert, что клетка (x, y) занята, а также вынести логику "а не выходим ли мы за границы поля" в этот метод, это упростит место использования

42 assert(sign && "try move empty sign");
сообщение можно немного улучшить, например, так:
"[Board::canMove] Empty sign is not allowed."

Сама проверка конечно же по делу, но лучше не полагаться на значения элементов в перечислении (зачем? что, если значения поменяются?), а использовать именованый вариант sign != EMPTY

16 if (x == -1 && y == -1)
17 return;

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

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

Сейчас существует ввод, который будет истрактован программой неверно (не в соответствии с условием). Проблема (весьма нетрививальная) в способе считывания. Попробуйте потестировать на большом количестве строк.

[Sokolov Viacheslav]

2 #include <ncursesw/ncurses.h>

так подключится wide-версия

текущее отображение поля не соответствует тому, что просят в условии (см. пример)

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

Какая мотивация реализовывать методы прямо в заголовочном файле?

Формально инвариантом еще является _cntX >= 0, _cntO >= 0

Лучше переменные, которые не предполагается модифицировать, помечать как const, что особенно актуально для случая static.

Можно использовать std::array, тогда не будет нужды использовать константу 4, что убережет от возможных ошибок

12 std::optional<Board::Player> sign;
player?

main:
по коду curses и std принципиально по-разному существуют, чего было бы хорошо избежать

Хотя abort в целом лучше не использовать, в данном случае, думаю, уместно.
(альтернативно можно было бы кидать исключения и в main принимать решение об остановке)

Возможно, использование Position в Board упростило бы кодовую базу.

[Sokolov Viacheslav]

40 boardStr += "0";

В условии просили все же O