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

Snakes Game Using ncurses

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

Problem

This is my version of snakes in C++ using the ncurses library. I would like to hear from you how this piece of code can be improved and general advice for future projects regarding coding and efficiency. I have tried writing this code in pure OOP, let me know about that too.

```
#include
#include
#include
#include

class snake //to store the x and y coordinates of each snake part
{
int x, y;
char ch;
public:
snake(){ x= y= 0; ch='O';}
snake(int a, int b)
{
x= a; y= b; ch= 'O';
}
snake(const snake &ekans)
{
ch= ekans.ch;
x= ekans.x;
y= ekans.y;
}
void setCh(char x)
{
ch= x;
}
char getCh()
{
return ch;
}
int getX()
{
return x;
}
void setX(int no)
{
x= no;
}
int getY()
{
return y;
}
void setY(int no)
{
y= no;
}
};

class list //to store each snake body
{
node* head;
int length;
public:
list()
{
head= NULL;
length= 0;
}
void add(snake s) //adds at the end
{
node *n= new node;
n->setSnake(s);
n->setNext(head);
head= n;
length++;
}
int listLength()
{
return length;
}
snake get(int n)
{
node *temp =head;
int count= 1;
while(count!= n && temp!= NULL)
{
count++;
temp= temp->getNext();
}
return temp->getSnake();
}
void remove() //removes the first element
{
node *temp= head->getNext();
node *t2= head;
while(temp->getNext()!=NULL)
{
temp=temp->getNext();
t2= t2->getNext();
}
t2->setNext(NULL);
delete temp;
length--;
}
void display()
{
int i= 0;
node *temp= head;
wh

Solution

if(b)
        return false;
    else
        return true;


With the ! operator, you can negate a boolean value.

So by saying !b, you can swap the cases around:

if(!b)
        return true;
    else
        return false;


But then you have "if it is true that b is not true, return true, else, return false". At that point, you might as well return "whether it is true that b is not true".

Like so:

return !b;


Aside from that, maybe b isn't really the right name. How about collided? Or maybe collisionFound.

bool game:: check()
{
    bool collisionFound= false;
    snake s= l.get(1);
    int head_x= s.getX();
    int head_y= s.getY();
    if(head_x== max_x-1 || head_y== 1 || head_x== 1 || head_y== max_y-1)
        return false;
    else 
    {
        int len= l.listLength();
        for(int i= 4; i< len; i++)
        {
            s= l.get(i);
            if(head_x== s.getX() && head_y== s.getY())
            {
                collisionFound = true;
                break;
            }
        }
        return !collisionFound;
    }  
}


Maybe you should label the function checkCollision. It's weird that you're returning a negative, though. Let's see where you're using this...

b= check();
if(!b)


Huh...

There's no need to store to b here, you can just say if(!checkCollision()). But that looks weird, game-over if not collision. It's because you've made check more of a check if snake is okay function, but it really checks for collisions and then negates the result. So you'd be better off by removing the negation:

bool game:: checkForCollision()
{
    bool collisionFound= false;
    snake s= l.get(1);
    int head_x= s.getX();
    int head_y= s.getY();
    if(head_x== max_x-1 || head_y== 1 || head_x== 1 || head_y== max_y-1)
        return true;
    else 
    {
        int len= l.listLength();
        for(int i= 4; i< len; i++)
        {
            s= l.get(i);
            if(head_x== s.getX() && head_y== s.getY())
            {
                collisionFound = true;
                break;
            }
        }
        return collisionFound;
    }  
}


And then using like so:

if(checkForCollision())
   {
        clear();
        mvprintw(max_y/2-2, max_x/2-8, "GAME OVER");
        mvprintw(max_y/2, max_x/2-10, "YOUR SCORE %d",score);
        refresh();
        sleep(2);
        break;
    }


If you make enough use of descriptive function names and variable names, eventually the code will read like a weird form of English.

Here you start checking snake segments starting from segment 4.

int len= l.listLength();
    for(int i= 4; i< len; i++)


I get the logic behind it: Snake is shaped like so:

. = not snake
o = snake head
^<>v = snake body

....
.v<.
.o^.
....


It's not possible to have a collision with indexes 1, 2 and 3, right? Only a snake of a head and 4 segments or longer could collide, so why not save some performance by only checking collisions from segment 4 and onwards?

Except... what happens if we put the snake in reverse?

You get one crumpled up snake.

....
.ô<.
.^..
....


Poor ekans.

Jokes aside, you should come up with something for this case. Some snake games I played would give me an instant gameover, and some snake games I played wouldn't allow me to move backwards (because snake cannot move backwards). Not allowing the player to move backwards might be more friendly to the player, because if the player presses the wrong button they might ekans by accident.

case KEY_UP: direction= 1; 
                    break;
        case KEY_DOWN: direction= 3;
                    break;
        case KEY_RIGHT: direction= 2;
                    break;
        case KEY_LEFT: direction= 4;
                    break;


Imagine you didn't have KEY_UP, KEY_DOWN, KEY_RIGHT and KEY_LEFT. Your code would look like this:

case 38: direction= 1; 
                    break;
        case 40: direction= 3;
                    break;
        case 39: direction= 2;
                    break;
        case 37: direction= 4;
                    break;


That'd be hard to understand, wouldn't it!

Except... you've kinda done the same thing with direction. You should try defining constants for directions. Let's use an enum for this.

So define your directions at the top of your code:

enum Direction { UP, DOWN, LEFT, RIGHT };


Declare the game's direction variable to be a Direction:

Direction direction;


And use like so:

case KEY_UP: direction= UP; 
                    break;
        case KEY_DOWN: direction= DOWN;
                    break;
        case KEY_RIGHT: direction= RIGHT;
                    break;
        case KEY_LEFT: direction= LEFT;
                    break;


(if you keep your classes and enums in different files, you may need to access it with ::, like Direction::LEFT).

```
if(dir== UP) y--;
else if(dir== RIGHT) x++;
else if(dir== DOWN)

Code Snippets

if(b)
        return false;
    else
        return true;
if(!b)
        return true;
    else
        return false;
bool game:: check()
{
    bool collisionFound= false;
    snake s= l.get(1);
    int head_x= s.getX();
    int head_y= s.getY();
    if(head_x== max_x-1 || head_y== 1 || head_x== 1 || head_y== max_y-1)
        return false;
    else 
    {
        int len= l.listLength();
        for(int i= 4; i< len; i++)
        {
            s= l.get(i);
            if(head_x== s.getX() && head_y== s.getY())
            {
                collisionFound = true;
                break;
            }
        }
        return !collisionFound;
    }  
}
b= check();
if(!b)
bool game:: checkForCollision()
{
    bool collisionFound= false;
    snake s= l.get(1);
    int head_x= s.getX();
    int head_y= s.getY();
    if(head_x== max_x-1 || head_y== 1 || head_x== 1 || head_y== max_y-1)
        return true;
    else 
    {
        int len= l.listLength();
        for(int i= 4; i< len; i++)
        {
            s= l.get(i);
            if(head_x== s.getX() && head_y== s.getY())
            {
                collisionFound = true;
                break;
            }
        }
        return collisionFound;
    }  
}

Context

StackExchange Code Review Q#121192, answer score: 19

Revisions (0)

No revisions yet.