4
\$\begingroup\$

To give you a simple background, I've been programming in C++ for about a week and decided I wanted to build a simple snake game.

I made one over the course of a day and this is the end result. What overall improvements can be made to this code? Are there any general advice on programming that might be relevant?

#include <iostream>
#include <map>
#include <cstdlib>
#include <conio.h>
#include <cmath>
#include <vector>
#include <ctime>

using namespace std;

const int boardX = 40, boardY = 20;
int center = ceil ((boardX/2*boardY)-(boardX/2));

map<int, char> gameMap;
map<int, char>::const_iterator it;

int snakeXY = center;
enum snakeDir {UP,DOWN,LEFT,RIGHT};
snakeDir direction;
vector<int> tailPieces;

bool fruitSpawned=false;
bool stopped;
bool gameOver;

// Creates and places all the elements into the map (gameMap)
void createMap(){
    for(int i = 1; boardX*boardY>=i; i++){ 
        if((i<=boardX) || (i>(boardX*boardY-boardX))){ 
            gameMap[i] = '#';
        } else if((i % boardX == 0) || ((i-1) % boardX == 0)){ 
            gameMap[i] = '#';
        } else {
            gameMap[i] = ' ';
        }
    }
}

// Handles input from the user
void keyUpdate(){
    char key;

    if(kbhit() == true){
        key = getch();

        switch(key){
            case 'w':
            case 'W':
                direction = UP;
                break;
            case 'a':
            case 'A':
                direction = LEFT;
                break;
            case 's':
            case 'S':
                direction = DOWN;
                break;
            case 'd':
            case 'D':
                direction = RIGHT;
                break;
            case 'P'
            case 'p'
                if(!stopped){
                    stopped = true;
                } else {
                    stopped = false;
                }
            default:
                break;
        }
    }
}

// Handles hit detection, snake's head and tail movement
void updateSnake(){
    int prevXY = snakeXY;

    if(!stopped){
        switch(direction){
            case UP:
                snakeXY = snakeXY - boardX;
                break;
            case DOWN:
                snakeXY = snakeXY + boardX;
                break;
            case LEFT:
                --snakeXY;
                break;
            case RIGHT:
                ++snakeXY;
                break;
        }

       if(!stopped){
            if(tailPieces.empty()){
                gameMap[prevXY] = ' ';
            } else {

                int tailPieceXY;

                for(int i = 0; i <= tailPieces.size(); i++){
                    if (i == tailPieces.size()){
                        gameMap[prevXY] = ' ';
                    } else {
                        tailPieceXY = tailPieces[i];
                        tailPieces[i] = prevXY;
                        gameMap[prevXY] = 'o';
                        prevXY = tailPieceXY;
                    }
                }
            }
            if(gameMap[snakeXY] == '#' || gameMap[snakeXY] == 'o'){
                gameOver = true;
            } else if(gameMap[snakeXY] == 'F') {
                tailPieces.push_back(prevXY);
                gameMap[snakeXY] = 'O';
                fruitSpawned = false;
            } else {
                gameMap[snakeXY] = 'O';
            }
        }
    }
}

// Draws the map into the cmd console.
void updateMap(){
    system("cls");
    for (it = gameMap.begin(); it!= gameMap.end(); it++){
            cout << it->second;
        if(it->first % boardX == 0){
            cout << endl;
        }
    }
}

// Determines locations for the fruit, and spawns them accordingly.
void spawnFruit(){

    vector<int> safeSpace;

    for (it = gameMap.begin(); it!= gameMap.end(); it++){
        if(it->second == ' '){
            safeSpace.push_back(it->first);
        }
    }

    srand(time(NULL)); // Seeding for randomness
    int random = rand() % safeSpace.size();
    gameMap[safeSpace[random]] = 'F';
    fruitSpawned = true;

}


int main(){
    createMap();

    gameOver = false;
    stopped = false;

    while(!gameOver){
        updateMap();
        updateSnake();
        if(!fruitSpawned){
            spawnFruit();
        }
        keyUpdate();
        _sleep(125);
    }


    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ There are some good suggestions here. As your code seems fairly similar to the code in that question, you might find some of the answers useful. \$\endgroup\$ – Ethan Bierlein May 18 '16 at 2:54
5
\$\begingroup\$

Bugs

Line 61 and onward:

case 'd':
case 'D':
    direction = RIGHT;
    break;
    case 'P'
        case 'p'
        if (!stopped) {
            stopped = true;
        }
        else {
            stopped = false;
        }
        default:
            break;
}

case 'P' and case 'p' are missing a : at the end. Without them your code will not compile.

Also keep in mind your indentation is off. The correct implementation would be:

case 'd':
case 'D':
    direction = RIGHT;
    break;
case 'P':
case 'p':
    if (!stopped) {
        stopped = true;
    }
    else {
        stopped = false;
    }
default:
    break;
}

Deprecations

In that same function, you're using kbhit() and getch(). Those are deprecated IIRC. Use the underscored versions instead, like this:

if (_kbhit() == true) {
    key = _getch();

Your sleep function is also deprecated. Since you're developing for Windows-only, consider using #include <windows.h> and use Sleep(125); in your main.

Another deprecation is the return 0; at the end of your main. It will be added by the compiler under the hood, no need to state it explicitly. This is since C++98.

Namespaces

using namespace std; is considered bad practice. Short code is not a requirement in C++, clear code is preferred. It's a thing commonly taught to new C++ programmers because it's 'easier', but it will royally bite you in the behind when conflicts arise.

Input

keyUpdate();

That's how you get your input. But that's not how you want to get your input. You want to handle your input real-time. You want an EventHandler to handle a KEY_EVENT. Now you can handle your input non-blocking and decouple the FPS from the main loop :-)

\$\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.