Console Ultimate Tic Tac Toe game

Last week I wrote a C++ program on Ultimate Tic Tac Toe. The issue was that the program was a bit lengthy (~230 lines) and it is needed to be around 150 lines.

I absolutely am not asking you to go through the whole code. Just skimming through might reveal possible contractions to code, and tips in general to write short but readable C++ code.

Code

#include 
#include 
#include 
#include 

using namespace std;

inline int X (int pos) {
    return pos / 3;
}

inline int Y (int pos) {
    return pos % 3 - 1;
}

string rule(80, '_');

class Grid
{
private:
    char subgrid[3][3];
    char left, right;

public:
    Grid(){}
    void set (int x, int y, char cell);
    char get (int x, int y);
};

void Grid::set (int x, int y, char cell) {
    subgrid[x][y] = cell;
}

char Grid::get(int x, int y) {
    return subgrid[x][y];
}

class Game
{
private:
    Grid grid[3][3];
    char player;
    size_t cur;

public:
    Game();
    void display(); 
    void play();
    void input (int& g);
    bool checkWin (Grid grid);
    void showScore();
};

Game::Game() {
    for (size_t i = 0; i > g;
        if (g > 0 && g ';
                        right = '> s;
        if (s > 0 && s  ";
        cin >> input;
        switch (input) {
            case play:
                game.play(); break;
            case scores:
                //showScores();
                break;
            case quit:
                std::exit(0);
            default:
                error = 1;
        }
        system("cls");
    } while (error);

    system("pause");
    return 0;
}

Here are some things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. For this program, I'd advocate removing it everywhere and using the std:: prefix where needed.

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example, if your terminal supports ANSI Escape sequences, you could use this:

void cls()
{
    std::cout << "\x1b[2J";
}

Eliminate unused variables

The variable s in your Game::play() code is defined but never used. Also, left and right within Grid are never used. Since unused variables are a sign of poor code quality, you should seek to eliminate them. Your compiler is probably smart enough to warn you about such things if you know how to ask it to do so.

Use rational default constructors

If you provide a constructor for Grid that initializes its contents to all ., then the constructor for Game is shorter and much more readable.

Grid() : subgrid{'.','.','.','.','.','.','.','.','.'} {}

Eliminate global variables

In this case, the only global variable is rule which is only used once. I'd move it to within Game::display() and declare it like this:

static const std::string rule(80, '_');

Delegate more to the subclass

The Grid object is not doing very much. It could be assisting more in the display() and checkWin() tasks in particular.

Eliminate unused #includes

The cstdlib library is not required if you change std::exit() to simply return in main.

Eliminate unimplemented code

The showScore() code is missing and is never called anyway. It could simply be deleted, along with the associated case statement and menu option.

Use const where practical

Member functions that don't alter the underlying object should be declared const.

Use standard structures and algorithms

One important and useful way to simplify code is to make better use of existing library code. In particular, the Standard Template Library (STL) would be very helpful here. For instance, you could use a std:array instead of a plain C array to represent each grid. Internally, the representation could be std::array and translation from x and y coordinates could be done by member functions. As an example:

class Grid {
private:
    std::array subgrid;

public:
    Grid() : subgrid{'.','.','.','.','.','.','.','.','.'} {}
    void set (int i, char cell) { subgrid[i] = cell; }
    char get (int i) const { return subgrid[i]; }
    char get (int x, int y) const { return subgrid[x+3*y]; }
    bool checkWin(char player) const {
        // check for col and row wins
        for (int i=0; i = 0 && linenum < 3) {
            for (int i=0; i<3; ++i) {
                ret += get(i, linenum);
            }
        }
        return ret; 
    }
};

Now the Game::display() is much neater and smaller:

void Game::display()
{
    cls();
    static const std::string rule(80, '_');
    std::cout  " << grid[i+j].line(line) << " < ";
                } else {
                    std::cout << "   " << grid[i+j].line(line) << "   ";
                }
            }
            std::cout << "\n";
        }
        std::cout << "\n\n";
    }
}

Avoid breaking loops

Rather than use break to exit a loop, it's usually better to simply declare the actual loop exit at the top so that someone reading your code doesn't have to wonder where the actual exit lies. For example, one way to rewrite Game::input is like this:

void Game::input(int& g)
{
        int s;
        bool badinput = false;
        for (s = 0; s  9 || grid[g-1].get(s-1) != '.'; badinput = true) {
                display();
                if (badinput) {
                    std::cout > s;
        }
        grid[g-1].set(s-1, player);
        g = s;
}

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see I

StackExchange Code Review Q#145749, answer score: 8