Snake-like console game

Question

This is what I came up with when trying to make my first console game. I think it turned out quite okay so I wanted to know what you think about it. If there is anything to improve, may it be coding style, readability, efficiency or anything else, please let me know.

main.cpp

#include <iostream>
#include <stdlib.h>
#include <conio.h>
#include <windows.h>
#include "game.h"


int main()
{
    bool exit = false;

    std::cout   << "Controls:\n"
                << " - Use w, a, s, d to change direction.\n"
                << " - Press space to pause the game.\n\n";
    system("pause");
    system("cls");

    while( !exit )
    {
        size_t hight = 20, width = 20;

        Game game(hight , width);
        game.StartGame();
        while( !game.isGameOver )
        {
            game.Update();
            Sleep( 100 );
        }

        std::cout << "\nGame Over.\nPress 1 to play again.\nPress Esc to exit.";
        char input = 0;
        while( input != '1' && input != 27 )
        {
            input = getch();
        }

        if( input == 27)
            exit = true;

        system("cls");
    }

    return 0;
}

field.h

#ifndef FIELD_H
#define FIELD_H

#include <iostream>
#include <conio.h>

class Field
{
public:
    // Constructor & Destructor
    Field( size_t hight, size_t width );
    ~Field();

    // Methods
    void Draw();

    // Members
    char **field;
    enum BlockType{ EMPTY = ' ', WALL = '#', FOOD = '*', sHEAD = 2, sBODY = 'o' };

private:
    size_t HIGHT;
    size_t WIDTH;
};



#endif // FIELD_H

field.cpp

#include "field.h"

Field::Field( size_t hight, size_t width )
 : field( nullptr ), HIGHT( hight ), WIDTH( width )
{
    //  allocate field array
    field = new char* [HIGHT];

    for( size_t i = 0;  i < HIGHT; i++)
    {
        field[i] = new char[WIDTH];
    }

    // initialize field array
    for( size_t i = 0; i < WIDTH; i++ )
        field[0][i] = field[HIGHT - 1][i] = WALL;    // first and last line are borders

    for( size_t i = 1; i < HIGHT - 1; i++ )
    {
        field[i][0] = field[i][WIDTH - 1] = WALL;
        for( size_t j = 1; j < WIDTH - 1; j++ )     // first and last column are borders
        {
            field[i][j] = ' ';
        }
    }
}

Field::~Field()
{
    for( size_t i = 0; i < HIGHT; i++)
    {
        delete [] field[i];
    }

    delete [] field;
}

void Field::Draw()
{
    for( size_t i = 0; i < HIGHT; i++)
    {
        for( size_t j = 0; j < WIDTH; j++)
        {
            putch(field[i][j]);
        }
        putch('\n');
    }
}

snake.h

#ifndef SNAKE_H
#define SNAKE_H

#include <iostream>

class Snake
{
public:
    // Constructor & Destructor
    Snake( size_t MaxLength, size_t pos_x, size_t pos_y );
    ~Snake();

    // Members
    enum Direction{ UP = 'w', DOWN = 's', LEFT = 'a', RIGHT = 'd', UNDIRECTED };

    size_t **snake;
    size_t length;
    Direction direction;
    size_t temp_TailPosition[2];

    // Methods
    void ChangeDirection( Direction dir );
    void Move();
    void Grow();

private:
    size_t maxLength;
};


#endif // SNAKE_H

snake.cpp

#include "snake.h"

Snake::Snake( size_t MaxLength, size_t pos_x, size_t pos_y )
 : length( 3 ), direction( UNDIRECTED ), maxLength( MaxLength )
{
    // Allocate snake array
    snake = new size_t *[maxLength];

    for( size_t i = 0; i < maxLength; i++)
    {
        snake[i] = new size_t [2];
    }

    // Initialize snake array
    for( size_t i = 0; i < length; i++)
    {
        snake[i][0] = pos_x;
        snake[i][1] = pos_y;
    }

    // Initialize temp_TailPosition
    temp_TailPosition[0] = pos_x;
    temp_TailPosition[1] = pos_y;
}

Snake::~Snake()
{
    for( size_t i = 0; i < maxLength; i++)
    {
        delete [] snake[i];
    }

    delete [] snake;
}

void Snake::ChangeDirection( Direction dir )
{
    switch( dir )
    {
    case UP:
        if( direction != DOWN )
            direction = UP;
        break;

    case DOWN:
        if( direction != UP )
            direction = DOWN;
        break;

    case LEFT:
        if( direction != RIGHT )
            direction = LEFT;
        break;

    case RIGHT:
        if( direction != LEFT )
            direction = RIGHT;
        break;

    case UNDIRECTED:
        break;
    }
}

void Snake::Move()
{
    if( direction == UNDIRECTED )
        return;

    // Remember tail position
    temp_TailPosition[0] = snake[length - 1][0];
    temp_TailPosition[1] = snake[length - 1][1];

    // Move Body
    for( size_t i = length - 1; i > 0; i-- )
    {
        snake[i][0] = snake[i - 1][0];
        snake[i][1] = snake[i - 1][1];
    }

    // Move Head
    switch( direction )
    {
    case UP:
        snake[0][1] -= 1;
        break;

    case DOWN:
        snake[0][1] += 1;
        break;

    case LEFT:
        snake[0][0] -= 1;
        break;

    case RIGHT:
        snake[0][0] += 1;
        break;
    }
}

void Snake::Grow()
{
    length++;
    snake[length - 1][0] = temp_TailPosition[0];
    snake[length - 1][1] = temp_TailPosition[1];
}

game.h

#ifndef GAME_H
#define GAME_H

#include <iostream>
#include <conio.h>
#include <windows.h>
#include <stdlib.h>     // srand, rand
#include <time.h>       //  time

#include "snake.h"
#include "field.h"

class Game
{
public:
    Game( size_t hight, size_t width );


    // Members
    bool isGameOver;


    // Methods
    void StartGame();
    char GetInput();
    Snake::Direction GetDirection( char input );
    void Update();
    void Draw();

private:
    // Members
    Field FIELD;
    Snake SNAKE;
    size_t HIGHT;
    size_t WIDTH;
    size_t score;

    // Methods
    void SpawnFood();
    void PutSnake();
    void MoveCurser( size_t x, size_t y );
    void GameOver();
    void Pause();
};



#endif // GAME_H

game.cpp

#include "game.h"

Game::Game( size_t hight, size_t width )
 : isGameOver( false ),
   FIELD( hight, width ),
   SNAKE( hight * width, width / 2, hight / 2),
   HIGHT( hight ),
   WIDTH( width ),
   score( 0 )
{
    // Put Head
    size_t &headPos_x = SNAKE.snake[0][0];
    size_t &headPos_y = SNAKE.snake[0][1];

    FIELD.field[headPos_y][headPos_x] = Field::BlockType::sHEAD;

    // Generate random seed
    srand(time(NULL));

    // Spawn Food
    SpawnFood();
}

void Game::StartGame()
{
    Draw();
    char input = 0;

    while( input != 'w' && input != 'a' && input != 's' && input != 'd' )
    {
        input = GetInput();
    }

    Snake::Direction direction = GetDirection( input );
    SNAKE.ChangeDirection( direction );
    SNAKE.Move();
    PutSnake();
    Draw();
}

char Game::GetInput()
{
    if( kbhit() == true )
    {
        char input = getch();
        return input;
    }

    return 0;
}

Snake::Direction Game::GetDirection( char input )
{
    switch(input)
    {
    case 'w':
        return Snake::Direction::UP;

    case 'a':
        return Snake::Direction::LEFT;

    case 's':
        return Snake::Direction::DOWN;

    case 'd':
        return Snake::Direction::RIGHT;

    default:
        return Snake::Direction::UNDIRECTED;
    }
}

void Game::Update()
{
    char input = GetInput();
    if( input == ' ' )
        Pause();
    Snake::Direction direction = GetDirection( input );
    SNAKE.ChangeDirection( direction );
    SNAKE.Move();
    PutSnake();
    Draw();
}

void Game::Draw()
{
    MoveCurser( 0, 0 );
    FIELD.Draw();
    std::cout << "Score: " << score;
}

void Game::SpawnFood()
{
    size_t  x = 0,
            y = 0;
    while( FIELD.field[y][x] != Field::BlockType::EMPTY )
    {
        x = rand() % WIDTH;
        y = rand() % HIGHT;
    }

    FIELD.field[y][x] = Field::BlockType::FOOD;
}

void Game::PutSnake()
{
    size_t &headPos_x = SNAKE.snake[0][0];
    size_t &headPos_y = SNAKE.snake[0][1];
    size_t &oldTailPos_x = SNAKE.temp_TailPosition[0];
    size_t &oldTailPos_y = SNAKE.temp_TailPosition[1];


    switch( FIELD.field[headPos_y][headPos_x])
    {
    case Field::BlockType::FOOD:
        SNAKE.Grow();
        score++;

        FIELD.field[headPos_y][headPos_x] = Field::BlockType::sHEAD;        // Put Head
        for( size_t i = 1; i < SNAKE.length; i++ )                          // Put Body
        {
            FIELD.field[SNAKE.snake[i][1]][SNAKE.snake[i][0]] = Field::BlockType::sBODY;
        }

        SpawnFood();
        break;

    case Field::BlockType::EMPTY:
        FIELD.field[oldTailPos_y][oldTailPos_x] = Field::BlockType::EMPTY;  // Remove old Tail
        FIELD.field[headPos_y][headPos_x] = Field::BlockType::sHEAD;        // Put Head

        for( size_t i = 1; i < SNAKE.length; i++ )                          // Put Body
        {
            FIELD.field[SNAKE.snake[i][1]][SNAKE.snake[i][0]] = Field::BlockType::sBODY;
        }
        break;

    case Field::BlockType::WALL:
    case Field::BlockType::sBODY:
        isGameOver = true;
        GameOver();
        break;
    }
}

void Game::MoveCurser( size_t x, size_t y )
{
    HANDLE hOut;
    COORD position;

    hOut = GetStdHandle( STD_OUTPUT_HANDLE );

    position.X = x;
    position.Y = y;

    SetConsoleCursorPosition( hOut, position);
}

void Game::GameOver()
{
    size_t &oldHeadPos_x = SNAKE.snake[1][0];
    size_t &oldHeadPos_y = SNAKE.snake[1][1];

    for( char i = 0; i < 4; i++ )
    {
        MoveCurser( oldHeadPos_x, oldHeadPos_y);
        putch(Field::BlockType::sHEAD);
        Sleep(150);
        MoveCurser( oldHeadPos_x, oldHeadPos_y);
        putch(Field::BlockType::EMPTY);
        Sleep(150);
    }
}

void Game::Pause()
{
    system("cls");
    std::cout << "Game paused.\nPress space to continue.";
    char input = 0;
    while( input != ' ')
    {
        input = getch();
    }
    system("cls");
    Draw();
}

Answer 1

I have found a couple of things that could help you improve your code.

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";
}

Isolate platform-specific code

In this code, there are several things that are DOS/Windows only including #include <conio.h> and the getch() and kbhit() functions within that, and also system("cls"); that I've already mentioned. Your code runs successfully on Linux if I supply those missing functions, but it would be nice if there were an #ifdef WINDOWS already in the code so that one could recompile without having to alter the source code.

Consider using a better random number generator

You are currently using

x = rand() % WIDTH;

There are problems with this approach. One is that the low order bits of the random number generator are not particularly random, so neither is x. On my machine, there's a slight but measurable bias toward 0 with that. A better solution, if your compiler and library supports it, would be to use the C++11 `std::uniform_int_distribution. It looks complex, but it's actually pretty easy to use. Here's an example:

unsigned rand_uint(unsigned low, unsigned high)
{
    static std::default_random_engine re{};
    using Dist = std::uniform_int_distribution<unsigned>;
    static Dist uid{};
    return uid(re, Dist::param_type{low,high});
}

Eliminate "magic numbers"

There are a few numbers in the code, such as 20 and 27 that have a specific meaning in their particular context. By using named constants such as WIDTH or ESC, the program becomes easier to read and maintain.

Use correct spellings

The word "height" is misspelled as "hight" throughout the program. It's not a major point but it makes your code harder for others to use and understand if common words are not spelled correctly.

Use const where practical

The hight and width variables in main.cpp are never altered in the program and should be declared as const. Similarly, member functions that don't alter the underlying object, such as Field::Draw() should be declared as const.

Separate class responsibilities more completely

The Game class reaches into the Snake class and directly manipulates Snake member variables. This is a sign that the classes are not as well designed as they could be. Specifically, it seems that much of the code that is currently in Game::PutSnake() should be delegated instead to the Snake object. To take another specific example, consider rewriting the Game::SpawnFood() function to eliminate the need for direct access to the field member data:

void Game::SpawnFood()
{
    size_t  x = 0,
            y = 0;
    while( !FIELD.isEmpty(x,y) )
    {
        x = rand_uint(0, WIDTH);
        y = rand_uint(0, HIGHT);
    }
    FIELD.dropFood(x, y);
}

Carefully separate interface from implementation

The #include files that you put into a .h file are a part of its interface, while the ones in the corresponding .cpp file are part of the implementation. So for instance, conio.h is not required to understand and use the interface for Field and so that #include should not appear in the field.h file. It should instead be put into the field.cpp file since it's a detail of the implementation and not of the interface.

Omit return 0

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

Answer 2

Don't use raw pointers for your arrays. Instead use std::vector, which is no less efficient but much less prone to errors.

Additionally, to implement a 2D array, you should use a single array and do arithmetic to find the index instead of using an array of arrays.

Answer 3

    bool exit = false;

If you get rid of that line and change

    while( !exit )

to

    while ( true )

And then change

        while( input != '1' && input != 27 )
        {

to

        while ( input != '1' )
        {
            if ( input == 27 )
            {
                return EXIT_SUCCESS;
            }

You can get rid of the repetitive

        if( input == 27)
            exit = true;

You could also say break rather than return EXIT_SUCCESS, but I find this more direct in this case.

I would also agree with changing 27 to a constant but that's outside the point that I was making.

You may also want to clear the screen before returning. Since I switched to the block form of the if, that's as simple as adding a single line.