5
\$\begingroup\$

It's been a while since I last looked at C++ so before resuming one of my old projects I thought I'd brush up on the basics so I've written a basic noughts and crosses game (because it's just getting back into the language don't expect a fully fledged game, just a bit of working practice).

The game features (or lack thereof):

  • 2 Player only (i.e. no computer)
  • Separate grid and control descriptions (no numbers in free slots to represent possible moves)
  • No retry loop
  • Use of ANSI sequences to return to previous lines and overwrite the game grid between turns
  • Basic input processing
  • Separation of game logic from game loop
  • ... etc.

Specific things I'd like recommendations on:

  • The idea behind this was to allow clearing of the grid between moves, the reason for all the spaces is to remove the input character that would be present from the user selecting a move. This isn't needed in the other lines as they are overwriting the grid explicitly with a new grid. I'd like to be able to clear the grid more cleanly (or more portably, IIRC ANSI sequences aren't available on all systems?)
std::cout << "\033[F                                           \033[F\033[F\033[F\033[F\033[F";
  • Game::GetGrid uses a stringstream to create the grid for outputting, but I'd prefer some form of string interpolation (similar to C#s $"{grid[0][0]}"), but I couldn't immediately find any recommendations besides stringstreams.
  • Game::GetUserInput uses std::getline to read a line from the user and then checks the line length to determine if it's valid. I'd prefer to be able to use a function like _getch to get the user input but I know this isn't portable, what other options are there, or if there aren't any, did I do anything wrong in my method?

Compiled using MSVC with /std:C++14 /W3 /Od (plus other VS defaults) (no warnings reported).

Main.cpp

#include "Game.h"

#include <iostream>

int main() {
    Game game;
    bool finished = false;
    while (!finished)
    {
        std::cout << "\033[F                                           \033[F\033[F\033[F\033[F\033[F";
        std::cout << game.GetGrid();
        finished = game.MakeMove();
    }
    std::cout << "\033[F                                           \033[F\033[F\033[F\033[F\033[F";
    std::cout << game.GetGrid();
    std::cout << "Game Complete\n";
}

Game.h

#ifndef GAME_H
#define GAME_H

#include <vector>
#include <string>

class Game {
private:
    enum class Players {
        EMPTY,
        X,
        O
    };

    Players currentPlayer;
    std::vector<std::vector<Players>> grid;
    char GetCell(Players player);
    bool CanMove(int cellNo);
    void MakeMove(int cellNo);
    char GetUserInput();
    bool GameFinished();

public:
    Game();
    std::string GetGrid();
    bool MakeMove();
};

#endif // !GAME_H

Game.cpp

#include "Game.h"

#include <sstream>
#include <iostream>

Game::Game() {
    grid = {
        { Players::EMPTY, Players::EMPTY, Players::EMPTY },
        { Players::EMPTY, Players::EMPTY, Players::EMPTY },
        { Players::EMPTY, Players::EMPTY, Players::EMPTY }
    };
    currentPlayer = Players::X;
}

char Game::GetCell(Players player) {
    switch (player) {
    case Players::EMPTY:
        return ' ';
    case Players::X:
        return 'X';
    case Players::O:
        return 'O';
    }
}

bool Game::CanMove(int cellNo) {
    return grid[cellNo % 3][cellNo / 3] == Players::EMPTY;
}

void Game::MakeMove(int cellNo) {
    grid[cellNo % 3][cellNo / 3] = currentPlayer;
}

char Game::GetUserInput() {
    std::cout << "Enter cell number to make move (" << GetCell(currentPlayer) << "): ";
    std::string input;
    std::getline(std::cin, input);
    return input.length() == 1 ? input[0] : ' ';
}

bool Game::GameFinished() {
    // Horizontals
    if (grid[0][0] != Players::EMPTY && grid[0][0] == grid[1][0] && grid[1][0] == grid[2][0]) { return true; }
    if (grid[0][1] != Players::EMPTY && grid[0][1] == grid[1][1] && grid[1][1] == grid[2][1]) { return true; }
    if (grid[0][2] != Players::EMPTY && grid[0][2] == grid[1][2] && grid[1][2] == grid[2][2]) { return true; }

    // Verticals
    if (grid[0][0] != Players::EMPTY && grid[0][0] == grid[0][1] && grid[0][1] == grid[0][2]) { return true; }
    if (grid[1][0] != Players::EMPTY && grid[1][0] == grid[1][1] && grid[1][1] == grid[1][2]) { return true; }
    if (grid[2][0] != Players::EMPTY && grid[2][0] == grid[2][1] && grid[2][1] == grid[2][2]) { return true; }

    // Diagonals
    if (grid[0][0] != Players::EMPTY && grid[0][0] == grid[1][1] && grid[1][1] == grid[2][2]) { return true; }
    if (grid[2][0] != Players::EMPTY && grid[2][0] == grid[1][1] && grid[1][1] == grid[0][2]) { return true; }

    // Empty Cells
    for (int x = 0; x < 3; x++) {
        for (int y = 0; y < 3; y++) {
            if (grid[x][y] == Players::EMPTY) { return false; }
        }
    }

    return true;
}



std::string Game::GetGrid() {
    std::stringstream ss;
    ss << GetCell(grid[0][0]) << '|' << GetCell(grid[1][0]) << '|' << GetCell(grid[2][0]) << '\t';
    ss << 1 << '|' << 2 << '|' << 3 << '\n';
    ss << "-+-+-\t-+-+-\n";
    ss << GetCell(grid[0][1]) << '|' << GetCell(grid[1][1]) << '|' << GetCell(grid[2][1]) << '\t';
    ss << 4 << '|' << 5 << '|' << 6 << '\n';
    ss << "-+-+-\t-+-+-\n";
    ss << GetCell(grid[0][2]) << '|' << GetCell(grid[1][2]) << '|' << GetCell(grid[2][2]) << '\t';
    ss << 7 << '|' << 8 << '|' << 9 << '\n';
    return ss.str();
}

bool Game::MakeMove() {
    char move = GetUserInput();
    if ('1' <= move && move <= '9' && CanMove(move - '1')) {
        MakeMove(move - '1');
        currentPlayer = currentPlayer == Players::X ? Players::O : Players::X;
        return GameFinished();
    }
    return false;
}

Full solution repo


Example Playthrough

Note, this is not an exact representation of how the game plays out, each step shown below is after the user has entered their move but prior to pressing return, and as the screen clears between each move you wouldn't see all steps of the game at once, but they're included here as individual steps.

 | |    1|2|3
-+-+-   -+-+-
 | |    4|5|6
-+-+-   -+-+-
 | |    7|8|9
Enter cell number to make move (X): 5

 | |    1|2|3
-+-+-   -+-+-
 |X|    4|5|6
-+-+-   -+-+-
 | |    7|8|9
Enter cell number to make move (O): 3

 | |O   1|2|3
-+-+-   -+-+-
 |X|    4|5|6
-+-+-   -+-+-
 | |    7|8|9
Enter cell number to make move (X): 4

 | |O   1|2|3
-+-+-   -+-+-
X|X|    4|5|6
-+-+-   -+-+-
 | |    7|8|9
Enter cell number to make move (O): 6

 | |O   1|2|3
-+-+-   -+-+-
X|X|O   4|5|6
-+-+-   -+-+-
 | |    7|8|9
Enter cell number to make move (X): 9

 | |O   1|2|3
-+-+-   -+-+-
X|X|O   4|5|6
-+-+-   -+-+-
 | |X   7|8|9
Enter cell number to make move (O): 1

O| |O   1|2|3
-+-+-   -+-+-
X|X|O   4|5|6
-+-+-   -+-+-
 | |X   7|8|9
Enter cell number to make move (X): 2

O|X|O   1|2|3
-+-+-   -+-+-
X|X|O   4|5|6
-+-+-   -+-+-
 | |X   7|8|9
Enter cell number to make move (O): 8

O|X|O   1|2|3
-+-+-   -+-+-
X|X|O   4|5|6
-+-+-   -+-+-
 |O|X   7|8|9
Enter cell number to make move (X): 7

O|X|O   1|2|3
-+-+-   -+-+-
X|X|O   4|5|6
-+-+-   -+-+-
X|O|X   7|8|9
Game Complete
\$\endgroup\$
5
\$\begingroup\$

For a toy, it's completely fine to print whatever characters you want, because there's no need to consider portability. You make code that works on one computer and then it's done.

But then there's nothing really to respond to, so let's pretend this is a hypothetical first draft of a larger production.


The presentation and input of the game can be encapsulated separately out of the main function. That way if you run into some future difficulty using \033[F, it should be easier to understand which code needs porting because it will all be in once place.

The intent of this isn't just to make extra code for coding's sake, but to make the future work of porting as easy as possible, even if you don't presently know how it might need to be ported.

It also makes it easier to reason about various tasks because there's less unrelated code together in one place. It's not always obvious how to do this, but one rule is that the game logic is very unlikely to ever need any IO headers directly included.


Players should not have an EMPTY value. The current player can never logically be EMPTY, but this wouldn't be clear just from looking at the header.


Consider an array for the grid instead of a vector of vectors. It seems that they never need to be resized. Perhaps even making a Grid type (could be as simple as using Grid = std::array<...>;)


You have a grid of Players, which seems odd, because it's really representing the ownership of a slot. This is presumably why you added EMPTY to the enum. A simple alternative is to store a grid of std::optional<Players>. Or, an enum like SlotOwner which does have an EMPTY value inside it, but is distinct from the Players enum.


Consider making your private class methods file-private to the cpp file. For example, GetCell does not require any game context and therefore does not need to be a class method.


Consider overloading the ostream shift operator for Grid, or making the GetGrid method take an ostream argument. That way you don't need to construct a string only to put it directly into an ostream and then delete the string.

Same for GetUserInput and taking an istream.

This makes it more easily testable, flexible, uses less memory, and would be about the same amount of code.


Consider std::all_of for testing if the entire grid is empty.


Rename GetUserInput to something that indicates what it is returning.


Convert the result of GetUserInput to an int before using it to minimize the code that's treating it like a char. You want the reader to be able to forget about how the input was acquired as quickly as possible.


Rather than range checking the GetUserInput result in the MakeMove method, do the check inside CanMove which seems much more appropriate.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.