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

Simple endless obstacle dodging game

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

Problem

I've made a quick game in C++ where you race down a vertical track and dodge 'X's or obstacles. The player is represented as a 'v', and the score is counted by how far you manage to get down the track. The obstacles are randomly generated at the bottom of the map and slide up each time the player moves (using the arrow keys).

I have the dodgeGame() function linked from another source, so that's where the program actually starts.

How can I make it better?

#include 
#include 
#include 
#include 
#include 
using namespace std;

void restartMap(char map[9][9]) //Sets all values on map to ' '
{
        for(int i = 0; i |\nFinal Score: " << count << ".\n";
}

Solution

The first change I'd make would be to get rid of most of the 9s you currently have sprinkled throughout the code.

static const int rows = 9;
static const int cols = 9;


This way if you decide you want a 10x10 grid (or 8x11, etc.) it should be pretty easy to do that.

The second possibility would consider would be a (probably somewhat misplaced) attempt at optimization. In particular, instead of copying the rows up after each turn, I'd keep track of which row is the current starting point at any given time. Each update would just advance that, modify the row that's now the "bottom", and re-draw.

Honestly, as long as you're only dealing with an 9x9 grid, that's probably not going to gain enough to notice. For a game that (for example) was drawing 2D graphics instead, it would probably make a rather more meaningful difference.

I'd probably define names for the 80, 75 and 77 in the main loop as well:

enum {LEFT = 75, RIGHT = 77, DOWN = 80 };

// ...

switch (ch) { 
    case LEFT:
        if(player == 0) continue;
        if(map[1][player-1] == 'X')
        {
            crash = true;
            break;
        }
        map[0][player] = ' ';
        player--;
        map[0][player] = 'v';
        break;


The name player should probably also be changed to something like horizontal_position or current_column (at least I think that's what it represents).

You might also consider re-casting the code to be a map class, with reset (sounds more accurate than restart to me), draw, slide, and info as member functions. If you're going to do this, you might also consider getting rid of the reset completely. Instead, do that initialization in the constructor, and when you want to clear the grid, you just create a new map object.

One other minor detail: if(crash == true) should normally be written just as if (crash) instead.

Code Snippets

static const int rows = 9;
static const int cols = 9;
enum {LEFT = 75, RIGHT = 77, DOWN = 80 };

// ...

switch (ch) { 
    case LEFT:
        if(player == 0) continue;
        if(map[1][player-1] == 'X')
        {
            crash = true;
            break;
        }
        map[0][player] = ' ';
        player--;
        map[0][player] = 'v';
        break;

Context

StackExchange Code Review Q#121493, answer score: 4

Revisions (0)

No revisions yet.