Conversation
There was a problem hiding this comment.
Pull request overview
This PR restructures the codebase from a monolithic implementation to a fullstack-ready architecture, separating the game logic into a reusable backend library and creating a distinct CLI frontend. The restructuring prepares the project for future expansion with a web API and React frontend.
Key Changes:
- Separated backend game logic into a header-only library (
backend/include/model/) - Created standalone CLI application with proper CMake build configuration
- Added test data files and documentation for the CLI interface
Reviewed changes
Copilot reviewed 8 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/data/vertical-jane.in |
Test case for vertical win scenario |
tests/data/horizontal-jim.in |
Test case for horizontal win scenario |
tests/data/diag-faltint.in |
Test case for diagonal win with incorrect winner name in comment |
cli/view-controller.hpp |
Updated include paths to use backend library, removed trailing whitespace |
cli/main.cpp |
New CLI entry point |
cli/README.md |
CLI documentation with incorrect test file paths |
cli/CMakeLists.txt |
CLI build configuration |
build_cli.sh |
Build script for CLI application |
backend/include/model/win-line.hpp |
Extracted WinLine/WinPoint data structures with spelling error and incorrect copy assignment |
backend/include/model/tictactoe-model.hpp |
Refactored game model with constructor initialization order issue and performance concern |
backend/CMakeLists.txt |
Backend library configuration |
CMakeLists.txt |
Root CMake configuration requiring C++20 |
.gitignore |
Added build directory |
README.md |
Updated to reflect fullstack architecture |
Comments suppressed due to low confidence (2)
backend/include/model/tictactoe-model.hpp:98
- The
get_board()method returns the board by value, which creates an unnecessary copy of the entire unordered_map. This could impact performance, especially for large game states. Consider returningconst board_type&instead to avoid the copy.
backend/include/model/tictactoe-model.hpp:91 - The constructor's member initializer list has parameters in the wrong order. The member initialization order should match the declaration order in the class. Members are initialized in declaration order (tiles_to_win, player_names, turn, board, winner, dirs, line), but the initializer list has: turn, tiles_to_win, winner, player_names. This can lead to subtle bugs and should be reordered to: tiles_to_win(tiles_to_win), player_names({player1_name, player2_name}), turn(1), winner(-1).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
backend/include/model/tictactoe-model.hpp:99
- The
get_board()method is markednoexcept, but it returns a copy ofstd::unordered_map, which can throw during copy construction (e.g., if memory allocation fails). Remove thenoexceptspecifier or return a const reference instead.
backend/include/model/tictactoe-model.hpp:91 - Member initialization order doesn't match declaration order. Members are declared as:
tiles_to_win,player_names,turn,board,winner, but initialized as:turn,tiles_to_win,winner,player_names. While this will work, it's better practice to initialize in declaration order to improve maintainability and avoid potential issues.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Sample run | ||
|
|
||
| ``` | ||
| g++ -std=c++20 main.cpp && ./a.out |
There was a problem hiding this comment.
The documentation shows building with g++ -std=c++20 main.cpp && ./a.out, but the restructured codebase uses CMake and the code now depends on the backend library. This build command will not work. Consider updating to reference the CMake build process or the build_cli.sh script.
| g++ -std=c++20 main.cpp && ./a.out | |
| # Build and run using the provided script: | |
| ./build_cli.sh | |
| ./cli |
There was a problem hiding this comment.
only build_cli.sh is necessary :)
I will change
TODO