4
\$\begingroup\$

Here's my approach:

#include <iostream>
#include <ctime>
#include <cstdlib>
#include <windows.h>
#include <vector>
#include <conio.h>

using namespace std;

class snake {
private: int x, y,tail,apple_x,apple_y,dx,dy,lastpos_x,lastpos_y,points,level;
         bool go,eaten;
         char**map;

//vectors to store the tail of the snake

         std::vector<int> x_tail;
         std::vector<int> y_tail;
public: snake();
        ~snake();
        void start_game();
        void game_over();
        void move();
        void input();
        void apple();
        void game();
        void refresh();
        void clear();
        void draw();
        void hidecursor();
        void test();
        int check_over();
};

//hide cursor function

void snake::hidecursor()
{
    HANDLE consoleHandle = GetStdHandle(STD_OUTPUT_HANDLE);
    CONSOLE_CURSOR_INFO info;
    info.dwSize = 100;
    info.bVisible = FALSE;
    SetConsoleCursorInfo(consoleHandle, &info);
}

//test function

void snake::test() {
    for (int i = 0; i <= 5; i++) {
        x_tail.push_back(i);
    }
    cout << endl;
    x_tail.insert(x_tail.begin(), 100);
    for (unsigned i = 0; i < x_tail.size(); i++) {
        cout << x_tail[i] << " ";
    }
}

//initial values

snake::snake() {
    go = true;
    eaten = false;
    level = 100;
    dx = 0;
    dy = 1;
    x = 6;
    y = 25;
    apple_x = 1;
    apple_y = 1;
    tail = 3;
    points = 0;
}

snake::~snake() {
    for (int i = 0; i < 26; i++) {
        delete[] map[i];
    }
    delete[] map;
}

//start the game

void snake::start_game() {
    hidecursor();
    map = new char*[26];
    for (int i = 0; i < 26; i++) {
        map[i] = new char[81];
    }
    draw();
}

//draw map

void snake::draw() {

//draw top and bottom walls

    for (int j = 1; j <= 80; j++) {
            map[1][j] = char(178);
            map[25][j] = char(178);
        }

//draw right and left walls

    for (int i = 1; i <= 25; i++) {
            map[i][1] = char(178);
            map[i][80] = char(178);
    }
    for (int i = 2; i <= 24; i++) {
        for (int j = 2; j <= 79; j++) {
            map[i][j] = ' ';
        }
    }
//draw head and tail of the snake

    map[x][y] = char(254);
    for (int i = 22; i <= 24; i++) {
        y_tail.push_back(i);
    }
    for (int i = 0; i < tail; i++) {
        x_tail.push_back(x);
    }
    for (int i = 0; i < tail; i++) {
        map[x_tail[i]][y_tail[i]] = 'o';
    }

//display the entire map

    for (int i = 1; i <= 25; i++) {
        for (int j = 1; j <= 80; j++) {
            if (j == 40)
                cout << map[i][j] << endl;
            else
                cout << map[i][j];
        }
    }

//generate apple
    apple();

//movement
    move();
}

void snake::refresh() {

//update map with the new positions

    map[x][y] = char(254);
    for (int i = 0; i < tail; i++) {
        map[x_tail[i]][y_tail[i]] = 'o';
    }

//display the updated map

    for (int i = 1; i <= 25; i++) {
        for (int j = 1; j <= 80; j++) {
            if (j == 80)
                cout << map[i][j] << endl;
            else
                cout << map[i][j];
        }
    }

//continue to detect movement after update
    move();
}

//apple random generation
void snake::apple() {
    srand((unsigned)time(NULL));
    apple_x = 2 + rand() % (24 - 2 + 1);
    apple_y = 2 + rand() % (79 - 2 + 1);
    map[apple_x][apple_y] = 'o';
}

//check if the snake hit a wall or himself

int snake::check_over() {
    if ((x == 1) || (x == 25) || (y == 1) || (y == 80)) {
        return 1;
    }
    for (int i = 0; i < tail-1; i++) {
        if ((x_tail[i] == x) && (y_tail[i] == y))
            return 1;
    }
    return 0;
}

//game over...

void snake::game_over() {
    system("cls");
    Sleep(1000);
    cout << "Game Over!!!" << endl;
    Sleep(50);
    cout << "You earned " << points << " points!!!" << endl;
}

void snake::clear() {
    system("cls");
}

//snake movement

void snake::move() {

//repeat movement until game over
    while (go == true) {

//update directions of the snake if a key is pressed... if not keep previous directions

        if (_kbhit()) {
            input();
        }
//update head position

        cout << endl;
        lastpos_x = x;
        lastpos_y = y;
        x = x + dx;
        y = y + dy;

//generate new apple and increase tail if apple is eaten
        if ((x == apple_x) && (y == apple_y)) {
            points = points + 10;
            eaten = true;
            map[apple_x][apple_y] = ' ';
            apple();
        }

        if (eaten == true) {
            tail++;
            x_tail.insert(x_tail.begin(), x_tail[0]);
            y_tail.insert(y_tail.begin(), y_tail [0]);
            eaten = false;
        }

//build tail

        map[x_tail[0]][y_tail[0]] = ' ';
        for (int i = 0; i < tail-1; i++) {
            x_tail[i] = x_tail[i+1];
            y_tail[i] = y_tail[i+1];
        }
        x_tail[tail - 1] = lastpos_x;
        y_tail[tail - 1] = lastpos_y;
        if (check_over()) {
            game_over();
            go = false;
            break;
        }

//increase velocity depending on the points earned

        if (points > 50 && points <= 100) {
            level = 75;
        }
        else if (points > 100 && points <= 150) {
            level = 50;
        }
        else if (points > 150) {
            level = 25;
        }
        Sleep(level);

//clear the screen and update the map

        clear();
        refresh();
    }
}

//keyboard input

void snake::input() {
    if (_kbhit()) {
        char in = _getch();
        switch (in) {
            case 'd':
            {
                dx = 0;
                dy = 1;
                break;
            }
            case 'w':
            {
                dx = -1;
                dy = 0;
                break;
            }
            case 'a':
            {
                dx = 0;
                dy = -1;
                break;
            }
            case 's':
            {
                dx = 1;
                dy = 0;
                break;
            }
        }
    }
}


void main() {
    snake game;
    game.start_game();
    system("pause");
 }

How can I make this program more efficient? Also when the snake gets bigger (tail over 20 blocks) , the game crashes or the apple generation is really slow.

P.S: the test function has nothing to do with the program... i just test stuff there before writing in the main functions of the class.

\$\endgroup\$
6
  • \$\begingroup\$ #include <conio.h> means very few people will be able to help you. Writing code for MS-DOS compilers just isn't what people do anymore. \$\endgroup\$
    – nwp
    Nov 14 '17 at 16:51
  • \$\begingroup\$ ok... I know it's obsolete....alternatives for kbhit and getch? \$\endgroup\$ Nov 14 '17 at 17:01
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Mast
    Nov 27 '17 at 16:40
  • \$\begingroup\$ sorry... I'll keep in mind for future posts. \$\endgroup\$ Nov 27 '17 at 16:52
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Last warning before I flag for moderator attention. \$\endgroup\$
    – Mast
    Nov 28 '17 at 9:23
3
\$\begingroup\$

Design

  • There are multiple uses of implicit x, y coordinate pairs. These could easily be encapsulated in a simple point (or vector2) struct to explicitly express that relation.
  • Some functions have hidden caveats/multiple responsibilities:
    • I wouldn't expect snake::draw() to run the game.
    • I wouldn't expect snake::move() to contain the actual game loop, or to handle player input.
  • Some functions work on inherently different levels of abstraction: hidecursor is rather low-level, whereas start_game is a rather high-level abstraction. These could be split into several classes with clear-cut scopes and same level of abstraction.

Naming

Many names, while not completely off the mark, are somewhat misleading. Some examples:

  • level is actually the delay between updates in milliseconds (as an arbitrary indicator of difficulty). update_delay or update_rate might be more fitting.
  • go is a boolean value indicating whether the game is still ongoing. is_running might be a better choice.
  • map might be conflated with the type std::map (especially since using namespace std; was declared).
  • The snake class represents the snake game, not a snake in the game.

Game play

  • There is an awful lot of flickering (at least on my machine). I'm not exactly sure what causes these flickers, probably some interference from the different console APIs and flushing too often (remember: every std::endl flushes the console output).
  • The game can instantly be lost by pressing the key for the direction the snake is coming from (e.g. pressing S while the snake is moving up). This can easily happen when trying to do a fast maneuver (e.g. A + S to make a quick turn).
  • Apples can spawn inside the snake body. Once the snake leaves the tile, it gets overwritten with a space, so it seems like there is no apple on the field (though it's still there and can be eaten, it's just invisible).
  • A lot of the timing for the different difficulty levels is depending on/gets outclassed by the time of system("cls") in snake::clear().
  • The whole level border is hard-coded. It could be generated/selected dynamically for different game modes (e.g. no border with wrap-around snake, or advanced maze-like borders for higher difficulty).
  • User input feels sluggish: It is only handled after the next frame has been drawn (so the snake continues to move into the old direction for 1 more turn). Additionally, there is the option of chaining user inputs, as only the oldest input gets processed, which might be a annoying (cannot change direction quickly enough if too many inputs in buffer) or a feature (can send inputs for precise maneuvers in advance).

Implementation

  • There are 4 different APIs used for console input/output (<conio.h>, <windows.h>, system calls and <iostream>). Maybe use one exclusively for consistency?
  • snake::game() is declared, but not implemented or used.
  • There is a stack overflow: snake::move() calls snake::refresh(), which in turn calls snake::move() until the call stack overflows. This can be fixed by simply removing the call to snake::move() in snake::refresh().
  • using namespace std; is considered bad practice and should be avoided.
  • snake::~snake() unconditionally dereferences map - even if it might not be initialized yet. This can invoke undefined behavior!
  • snake::map is initialized to hold 26 arrays of 81 chars each, of which the first array and the first char of the other arrays are never actually used (they are only indexed starting from 1, unless they are allocated/deallocated). (Actually, why allocate map at all? One could easily replace it with an inplace char[25][80] array, or a std::vector<std::string>, or similar.)
  • Is it really necessary to print each char separately? With a bit of planning, the whole map could be printed in one call!
  • There are many "magic numbers" used in different parts of the code (e.g. assumed width and height of the console, char representations of different game elements, ...). Readability could be improved by using appropriately named constants instead.
\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much for your advice... it really helped me improve the program!!! \$\endgroup\$ Nov 27 '17 at 17:06

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.