3
\$\begingroup\$

I wrote a Snake Game in C++ and I have practiced some OOP approaches on windows 7.

What I need:

  1. advice how to improve code
  2. Most important how to made what Snake on CollisionCheck method do different "things" for example if snake eats Fruit snake will grow by one. if Snake eats self or wall .game collapse etc/
  3. Any ideas how to add functional what you can make maps in notepad and import into the game?

Source.cpp

// Source.cpp : Defines the entry point for the console application.
//
#include <conio.h>
#include <iostream>
#include "stdafx.h"
#include"Snake.h"
using namespace std;


int main()
{
    Snake snake(20, 20);
    Fruit fruit(10,10);
    Wall wall(15, 15);
    int i = 0;
    while (i < 800) {
        ++i;
        snake.Move();
        fruit.Draw();
        wall.Draw();
            if (snake.CollisionCheck(&fruit))break;
        Sleep(100);
    }

    std::cout << " END GAME! "<<std::endl;

    cout << snake.SnakeLength();
    return 0;
}


Snake.h

    #pragma once
#include <iostream> 
#include <vector>
#include "Box.h"
#include <conio.h>
using namespace std;
#define MAX_SNAKE_SIZE 500

class Snake 
{
public:

    Snake(int StartX, int StartY);

    ~Snake();
    void ShowSelf();
    void TurnRigth();
    void TurnLeft();
    void TurnUp();
    void TurnDown();
    void AddBox(); //adds element at the end of the snake
    void Move();
    bool CollisionCheck(Obstacle*object);
    int SnakeLength() { return box.size(); };
private:
    int x;
    int y;
    char dir;//current direction  of snake
    vector<Box> box;//no dinamic array because sweet pain
    int length;
};  

Snake.cpp

#include "stdafx.h"
#include "Snake.h"
#include <windows.h>
#include <conio.h>
//#define DEBUG
enum Object 
{
    BOX = 0,
    FRUIT=1,
    WALL=2
};

Snake::Snake(int StartX,int StartY)
{
    x = StartX;
    y = StartY;
    length = 1;
    Box obj1(x,y);
    //obj1.SetSymbol('S');
    box.push_back(obj1);
}

Snake::~Snake()
{
    #ifdef DEBUG
    for (int i = 0; i < box.size(); i++)
    {
        Box temp = box[i];
        temp.ShowCord();
    }
    #endif
}

void Snake::ShowSelf()
{
    for (unsigned int i = 0; i < box.size(); i++)
    {
        box[i].Draw();
    }
}

void Snake::TurnRigth()
{
    dir = 'd';
}

void Snake::TurnLeft()
{
    dir = 'a';
}

void Snake::TurnUp()
{
    dir = 'w';
}

void Snake::TurnDown()
{
    dir = 's';
}

void Snake::AddBox()
{
    int size = box.size();
    Box temp(box[size-1].GetX(), box[size-1].GetY());
    box.push_back(temp);
}

void Snake::Move()
{
    system("cls");
    //allows Snake to move
    for (int i = box.size()-1; i>0; i--)
    {
        box[i].SetCord(box[i - 1].GetX(), box[i - 1].GetY());
    }
    //check what kind of button was pressed
    char tempdir = dir;
    if(_kbhit())dir = _getch();
    if (dir == 'w') {
        if (tempdir == 's') { //stops snake move backwards 
            box[0].MoveDown();
            dir = tempdir;
        }
        else box[0].MoveUp();
    }
    else if (dir == 'd') {
        if (tempdir == 'a') { //stops snake move backwards /
            box[0].MoveLeft();
            dir = tempdir; 
        }
        else box[0].MoveRigth();
    }
    else if (dir == 's') {
        if (tempdir == 'w') { //stops snake move backwards 
            box[0].MoveUp();
            dir = tempdir;
        }
        else box[0].MoveDown();
    }
    else if (dir == 'a') {
        if (tempdir == 'd') { //stops snake move backwards 
            box[0].MoveRigth();
            dir = tempdir;
        }
        else box[0].MoveLeft();
    }

    for (unsigned int i = 0; i < box.size(); i++) //draw Snake
    {
        box[i].Draw();
    }

}

bool Snake::CollisionCheck(Obstacle *object) //hiting magick appiers
{
    int boxX = box[0].GetX(); int boxY = box[0].GetY();
    //check if snake ate self or not 
    for (int i = 1; i < box.size(); ++i)
    {
        if ((box[0].GetX() == box[i].GetX()) && (box[0].GetY() == box[i].GetY())) { return true; }
    }

    //check if it hitten anything else is hitted
    if ((object->GetX() == box[0].GetX())&&(object->GetY() == box[0].GetY()))
    {
        int obstacle = object->onCollision();
        if (obstacle == FRUIT) { //deletes old glyph //spawns object at new coord
            this->AddBox();
            return false;
        }
        /*else if (obstacle == BOX) 
        {
            return 0;
        }*/
        else if (obstacle == WALL) 
        {
            return 0;
        }
    }
    return false;
}


Box.h where all potentional elements of game is (walls,Fruit,snake elements)

#pragma once
#include <iostream>
#include <conio.h>
#include <iostream>
#include <windows.h>

class Obstacle
{
public:
     Obstacle(int x, int y);

    void SetX(int x)  { this->x = x; };

    void SetY(int y) { this->y = y; };

    int GetX() { return x; };

    int GetY() { return y; };

    void Draw();
    void Erase();
    void NewSpawn();//Spawns Obstacle at new coords
    virtual int  onCollision();//check if hited or not 
    void SetSymbol(char symbol) { this->symbol = symbol; };
protected:
    char symbol;
    int x;
    int y;
};

class Box :public Obstacle
{
public:
    Box(int x, int y) :Obstacle(x, y) { this->SetSymbol('o'); };
    void SetCord(int x, int y);
    void MoveUp();
    void MoveDown();
    void MoveRigth();
    void MoveLeft();
    int onCollision()override;
};


class Fruit :public Obstacle
{
public:
    Fruit(int x, int y) :Obstacle(x, y) { this->SetSymbol('F'); };
    int onCollision ()override;
};

class Wall :public Obstacle 
{
public:
    Wall(int x, int y) :Obstacle(x, y) { this->SetSymbol('#'); };
    int onCollision()override;
};

Box.cpp

#include "stdafx.h"
#include "Box.h"
#include <stdlib.h>
#include"time.h"

enum Object
{
    BOX = 0,
    FRUIT = 1,
    WALL = 2
};
//place symbol at specific coords in console
void gotoxy(int x, int y)
{
    COORD c = { x, y };
    SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), c);
}

void Box::SetCord(int x, int y)
{
    this->x = x;
    this->y = y;
}

void Box::MoveUp()
{
    y--;
}

void Box::MoveDown()
{
    ++y;
}

void Box::MoveRigth()
{
    x++;
}

void Box::MoveLeft()
{
    x--;
}

int Box::onCollision()
{
    return BOX;
}

Obstacle::Obstacle(int x, int y)
{
    this->x = x;
    this->y = y;
}

void Obstacle::Erase()
{
    gotoxy(x, y);
    std::cout << " ";
}

void Obstacle::Draw()
{
    gotoxy(x, y);
    std::cout << symbol;
}

void Obstacle::NewSpawn()
{
    srand(time(NULL));//need to fix that
    x = rand() % 80;
    y = rand() % 23;
}

int Obstacle::onCollision()
{
    return BOX;
}

int Fruit::onCollision()
{
    this->Erase();
    this->NewSpawn();
    return FRUIT;
}

int Wall::onCollision()
{
    return WALL;
}

\$\endgroup\$
2
\$\begingroup\$

First on code review we prefer to see code without debug statements or commented out code because that means the code may not be ready for review.

The Good
The class Obstacle seems to be a well defined object, it could be an abstract class if the body for the virtual function onCollision() was not defined by Obstacle. This would force every class that inherits from Obstacle to define the function onCollision(). For a reference on object oriented programming you might want to look at SOLID programming which is a set of 5 programming principles that apply to objct oriented programming.

The main() function is nice and concise, however, you might want to implement a draw function that calls the object specific draw functions.

Avoid Using Namespace STD
If you are coding professionally you probably should get out of the habit of using the "using namespace std;" statement. The code will more clearly define where cout and other functions are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The function cout you may override within your own classes. This stack overflow question discusses this in more detail.

The using namespace std; really shouldn't be in header files (include files) at all. It can make tracing name collisions very difficult. Keep in mind that you may not always be the one maintaining the code.

The Usage of the this Pointer
Unlike some languages such as PHP, the this pointer is not required when accessing methods or variables within a class. Public methods or variables from other classes need to be referenced by the variable name of the object or CLASSNAME::MethodName() but within a class all methods public or private are known at compile and link time. The only time the this pointer might be necessary is when a variable local to the function has the same name as a variable declared in the class, such as in Obstacle::SetSymbol(char symbol).

Defining Constants in C++
There are a lot of Magic Numbers in the main() function (10, 15, 20, 100, 800), it might be better to create symbolic constants for all of them to make the code more readable and easier to maintain. These numbers may be used in many places and being able to change them by editing only one line makes maintenance easier. While you do have one symbolic constant defined in snake.h it might be better defined in a more modern way.

Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. The values for the variable k are defined by the problem, but it might be better to use symbolic constants rather than raw numbers in the switch statement. That would make the code easier to read and maintain. C++ provides a couple of methods for this, there could be an enum, or they could be defined as constants using const or constexpr. Any of these would make the code more readable. There is a discussion of this on stackoverflow.

Using #define CONSTANT VALUE is generally discouraged in C++, the use of const or constexpr is prefered over #define. #define is not type safe where const or constexpr is.

#define MAX_SNAKE_SIZE 500

versus

const int MAX_SNAKE_SIZE = 500;

One Set of Header and Source for Each Class
The files Box.h and Box.cpp define multiple classes, it would be easier to maintain the code if each class had it's own header and source file. Header files for classes that derive (inherit) from other classes can include the necessary header file. Some tools such as Visual Studio will add the #include statement when you use the class wizard to define a class with a dependency on another class.

The Obstacle class definition would definitely be better in it's own header file.

Use the Type std::size_t When Indexing Arrays

There are a number of for loops that uses a type int or unsigned int variable as the loop control variable and reference box.size(). When indexing into arrays or vectors, it is better to use the type std::size_t which is defined as unsigned int in both C and C++. All of the STD container class size functions return the type std::size_t, there is also another type which can be used std::ptrdiff_t. You can find a discussion of size_t versus ptrdiff_t in this stackoverflow question.

One of the benefits of using an unsigned index is that a negative number can not be used to index an array. Since arrays in C start at zero this is important and prevent bugs. A second possible benefit of using size_t is that it may prevent warning messages about type mismatches from some compilers when using functions such as std::container.size().

Other Type Mismatches
The function bool Snake::CollisionCheck(Obstacle *object) returns both Boolean values and integers, it would be better if it returned only Boolean values.

Bugs
The Snake class has a variable length that is initialized in the constructor but it is never referenced again in the program, it probably should be updated when the snake collides with food.

The Snake class has a function int SnakeLength() that return box.size() that should probably return snake.length (also type mismatch but the variable is being promoted from std::size_t to integer).

When in debug mode the Snake destructor performs non-destructor operations.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Well about it The Snake class variable length, I have suspicions what I do not need it because I am using vector for snake and I always can get "length" of the snake using vector methods 2)Why Fruit.Wall and Box classes are in one file because they are using "gotoxy" function(allows you to place symbol at specific place) and I tried to "split" project in separate project files and function just dies if I I have it in multiple files. 3)Main problem except others :) is how to make what different objects are passed to onCollision() method and they do different methods. \$\endgroup\$ – Inex Aug 4 '19 at 18:50
  • \$\begingroup\$ Make gotoxy() a member of Obstacle so that it gets inherited, otherwise put it in a namespace and a header file and include it where needed. I meant to mention that in my review. \$\endgroup\$ – pacmaninbw Aug 4 '19 at 19:11
  • \$\begingroup\$ What about ->)**"Main problem except others :) is how to make what different objects are passed to onCollision() method and they do different methods." **? \$\endgroup\$ – Inex Aug 5 '19 at 9:40
  • \$\begingroup\$ @Inex Can't answer that part here due to code review guidelines. You MIGHT get an answer on stackoverflow.com but clean up the code first. Code review guidelines are codereview.stackexchange.com/help/how-to-ask and codereview.stackexchange.com/help/dont-ask. \$\endgroup\$ – pacmaninbw Aug 5 '19 at 12:24

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.