HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Snake game in C++ for Windows console

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
snakegameforwindowsconsole

Problem

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
#include
#include
#include
#include
#include
#include

using namespace std;

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

map gameMap;
map::const_iterator it;

int snakeXY = center;
enum snakeDir {UP,DOWN,LEFT,RIGHT};
snakeDir direction;
vector 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*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

Solution

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 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 :-)

Code Snippets

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

Context

StackExchange Code Review Q#128553, answer score: 5

Revisions (0)

No revisions yet.