HW #2

[podoprigora.ivan]

first try.......

[Артур Гулецкий (huletski)]

Корректность

Самое главное замечание -- читайте, пожалуйста, внимательно условие, так как:

• ввод/вывод должен быть реализован, используя cstdio, а не iostream (на что указывает первая часть названия StdioBoardView);
• порядок вводимых координат/координаты модели перепутаны (сравните вывод решения с примером из условия);
• ход "-1 -1" должен приводить к завершению программы;
• valgrind находит ошибки;
• test.cpp должен хранить main, Test.cpp - реализацию базового класса Test.

Удостоверьтесь, что решение работает на примере из условия и valgrind не находит ошибок; перечитайте условие и удостоверьтесь, что программа реализована так, как требуется. Если сдаете решение с известными проблемами, пишите о них в комментарии.

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

Стиль

Код просмотрел по диагонали, так как он будет меняться. Замечания:

• Board.h: 7 - имя метода с заглавной зачем-то; публичные члены-данные; не используете enum (class)ы вообще (тип фигуры, статус игры); нужные методы не помечены как const;
• BoardTest?.h: имена классов почему-то со строчной; имена test case'ов должны описывать тестовый случай/функциональность, которая проверяется (e.g. test_move_1 -> put_O_free_cell);
• main.cpp:7. для чего эта строка?
• Board.cpp:7. memset?
• Board.cpp:10,11. Используйте enum classы, код читать невозможно (тем более, что используются hardcoded значения, а не константы);
• Board.cpp:18-21. Перепишите лаконичнее;
• StdioBoardView?.cpp:11. Модель _не должна_ себя печатать (если речь не идет о debug). Как отображать Model должен решать View (иначе зачем нужно разделение MVC?);
• StdioBoardView?.cpp:12. Красоту оценил, но зачем здесь она? Нужно дополнительно смотреть, что автор имел ввиду под ever (лезть в определение макроса, имя которого, кстати, обычно пишут заглавными символами);
• StdioBoardView?.cpp:28. выглядит так, что goto можно заменить на continue;
• StdioBoardView::start. Метод можно разделить на более мелкие: ввод данных; вывод данных о модели, победителе; цикл игры (Controller часть).

Перед следующей посылкой поищите в коде места, которые можно написать лаконичнее.

---

Баллы: корректность 4 (нужно соблюдать требования условия, решение работает не полностью), стиль 2 (код будет переписываться),

[podoprigora.ivan]

Исправил все замечания по корректности, valgrind вроде нигде ошибок не нашёл. Добавил бонусное задание и обработку некорректного ввода.

[Артур Гулецкий (huletski)]

Корректность

Неверно обрабатывается ход, состоящий из одного символа. Такой ход должен считается неверным:

{hw_02}[2156]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/podoprigora.ivan/hw_02
Updating '.':
At revision 2916.
{hw_02}[2157]$ make
mkdir -p obj
g++ -c -o obj/main.o -std=c++17 -Wall -Werror -Wextra -Iinclude  src/main.cpp -lncurses
g++ -c -o obj/Board.o -std=c++17 -Wall -Werror -Wextra -Iinclude  src/Board.cpp
g++ -c -o  obj/StdioBoardView.o -std=c++17 -Wall -Werror -Wextra -Iinclude  src/StdioBoardView.cpp 
g++ -c -o obj/NcursesBoardView.o -std=c++17 -Wall -Werror -Wextra -Iinclude  src/NcursesBoardView.cpp -lncurses 
g++ -o  hw_02 obj/main.o obj/Board.o obj/StdioBoardView.o obj/NcursesBoardView.o -lncurses
g++ -c -o obj/test.o -std=c++17 -Wall -Werror -Wextra -Iinclude  test/test.cpp
g++ -c -o obj/BoardTest.o -std=c++17 -Wall -Werror -Wextra -Iinclude  test/BoardTest.cpp
g++ -c -o obj/Test.o -std=c++17 -Wall -Werror -Wextra -Iinclude  test/Test.cpp
g++ -o  test_hw_02 obj/test.o obj/BoardTest.o obj/Board.o obj/Test.o
{hw_02}[2158]$ ./
hw_02       include/    obj/        src/        test/       test_hw_02
{hw_02}[2158]$ ./hw_02 

..........
..........
..........
..........
..........
..........
..........
..........
..........
..........
O move: 0
^C

Остальное работает, отлично.

Бонус

Не хватает информации о том, кто ходит в данный момент; было бы удобно, если бы выводилась информация о кнопках (пробел для хода, X для выхода).

Стиль

Board(.h/.cpp)

• публичные поля, нарушается инкапсуляция;
• в move, can_move информацию о том, какой игрок ходит, лучше передавать через определенный выше enum class Player (код Board.cpp:30,31 стал бы внятнее: if (sign != now_moving) { return false;} );
• hardcoded размер поля, нужно вынести в константу и использовать ее;
• в инициализации напрашивается memset;
• Board::move. В начале функции напрашивается assert(can_move(...));; 38: тернарный оператор; функциональность проверки закончилась ли игра лучше вынести в отдельную приватную функцию и подумать над тем, как ее упростить, так как много похожего кода;

StdioBoardView(.h/.cpp)

• .h. отступы
• .cpp:9. Старайтесь объявлять переменные ближе к месту первого использования;
• .cpp:14-21. verbose code, старайтесь писать код лаконичнее. Можно написать так:

  char player_label = model.now_moving == Player::O ? 'O' : 'X';
  printf("%c move: ", player_label);
  

• .cpp:22-46. ввод, его разбор и валидацию лучше вынести в отдельную функцию (e.g. read_move);
• .cpp:56. дублирование; можно уменьшить вложенность.
• .cpp:75. printf("\n");

NCursesBoardView(.h/.cpp)

• метод start делает все, нужно разбивать на более мелкие методы (e.g. setup_screen, handle_move и т.п., как минимум нужно разделить логику view и controller'a) + название метода странное: лучше уже run_game.

+ недочеты аналогичные недочетам StdioBoardView?

test.h

• пустой, так и должно быть?

Test(.h/.cpp)

• .cpp:11. отступы;
• .cpp:21. return !failedNum;

BoardTest(.h/.cpp)

• не хватило тестов, проверяющих различные конфигурации доски на предмет победы (диагонали, столбец, строка).

---

Баллы

• корректность: 19.5;
• bonus: 9;
• стиль: 4.

[Артур Гулецкий (huletski)]

Дедлайн.