Simple console-based car game

Question

Below is my code for a simple console base car game. Is this a good approach or am I missing anything?

CarG.h

#ifndef CARG_H
#define CARG_H


class CarG
{
    private:
        int sleep;
        int points;
        int n, n1;
        char cash, keyp;
    public:
        CarG();
        void gotoxy(int x, int y);
        void frame();
        void car(int r);
        void cls();
        void movement(char keyp);
        void lmove();
        void rmove();
        char cfun();
        void displayscore();
        void Ecar(int r, int x2);
        void Ecls(int r, int x2);
        char GameOver();
};

#endif // CARG_H

CarG.cpp

#include <iostream>
#include <conio.h>
#include <ctime>
#include <windows.h>
#include "CarG.h"
#define CC (x1==n && i==21) || (x1==n && i==22) || (x1==n && i==23) || (x1==n+1 && i==21) || (x1==n+1 && i==22) || (x1==n+1 && i==23) || (x1==n+2 && i==21) || (x1==n+2 && i==22) || (x1==n+2 && i==23) || (x1==n+3 && i==21) || (x1==n+3 && i==22) || (x1==n+3 && i==23) || (x1==n+4 && i==21) || (x1==n+4 && i==22) || (x1==n+4 && i==23)

using namespace std;

COORD coord={0,0};
 void CarG::gotoxy(int x,int y)
 {
    coord.X=x;
    coord.Y=y;
    SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE),coord);
 }

CarG::CarG()
{
    srand(time(0));
    cash = '$';
    n=27;
    points=0;
    n1=0;
    sleep=150;
}

 void CarG::frame()
 {
     for(int j=1;j<=3;j+=2)
     for(int i=0;i<=24;i++)
     {
         gotoxy(16*j,i);
         cout << char (178);
     }
 }

void CarG::car(int r)
 {

    n=n+r;
    gotoxy(n,22);
    cout << "@   @";
    gotoxy(n,23);
    cout << "| X |";
    gotoxy(n,24);
    cout << "@   @";
 }

void CarG::cls()
 {

    gotoxy(n,22);
    cout << "     ";
    gotoxy(n,23);
    cout << "     ";
    gotoxy(n,24);
    cout << "     ";
 }

void CarG::Ecar(int r, int x2)
{
    gotoxy(x2,r);
    cout << char(219) << "  " << char(219) ;
    gotoxy(x2,1+r);
    cout << char(219) << char(219) << char(219) << char(219) ;
    gotoxy(x2,2+r);
    cout << char(219) << "  " << char(219) ;
}

void CarG::Ecls(int r, int x2)
{
    gotoxy(x2,r);
    cout << "    ";
    gotoxy(x2,r+1);
    cout << "    ";
    gotoxy(x2,r+2);
    cout << "    ";
}

void CarG::movement(char keyp)
{
    switch(keyp)
    {
        case 'A':
        case 'a':
            lmove();
            break;
        case 'D':
        case 'd':
            rmove();
            break;
    }
}

void CarG::lmove()
{
        if(n<=17);
        else
        {   cls();  car(-2);    }
}

void CarG::rmove()
{
        if(n>=42);
        else
        {   cls();  car(+2);    }
}

char CarG::cfun()
{
    for(;;)
    {
        int x1 = rand() % 30 + 17;
        int x2 = rand() % 28 + 17;
        if(x1==x2)
            x1 = rand() % 31 + 17;

        for(int i=0;i<=23;i++)
        {
            gotoxy(x1,i);
            cout << ' ';
            gotoxy(x1,i+1);
            cout << cash;
            if(i==23)
            {
                gotoxy(x1,24);
                cout << ' ';
            }
            Ecar(i, x2);
            Sleep(sleep);
            for(int k=-3;k<=4;k++)
            for(int j=0;j<=3;j++)
            if((x2==n+k && i+2==21+j) || (x2==n+k && i+2==22+j) || (x2==n+k && i+2==23+j))
            {
                keyp=GameOver();
                return keyp;
            }
            Ecls(i, x2);
            if(CC)
            {
                car(0);
                points=points+5;
                break;
            }
            points++;
            displayscore();
            if(kbhit())
            {
                keyp=getch();
                if(keyp=='a' || keyp == 'A' || keyp=='D' || keyp=='d')
                {   movement(keyp);  }
                else if(keyp=='Q' || keyp=='q')
                {   return keyp;    }
                else
                {   continue;   }
            }
            if(points%100==0)
            {
                if(sleep>25)
                sleep-=25;
                else if(sleep>15 && sleep<25)
                    sleep-=3;
            }
        }
    }
}

void CarG::displayscore()
{
    gotoxy(50, 15);
    cout << "points = "<< points;
}

char CarG::GameOver()
{
    gotoxy(26,12);
    cout << "GAME OVER";
    gotoxy(23,13);
    cout << "Your Score is : " << points;
    gotoxy(19,15);
    cout << "Want To Play Again? Y/N: ";
    keyp=getche();
    if(keyp=='y' || keyp=='Y')
    {
        points=0;
        sleep=175;
        return keyp;
    }
    else if(keyp=='n' || keyp=='N')
    {   return 'q'; }
    else
    {   GameOver(); }

}

main.cpp

#include <iostream>
#include <conio.h>
#include <windows.h>
#include "CarG.h"

using namespace std;

int main()
{
    CarG a1;
    char kbh;
    do
    {
        system("cls");
        a1.frame();
        a1.gotoxy(20,21);
        a1.car(0);
        getch();
        kbh=a1.cfun();
    }
    while(kbh!='q');

    a1.gotoxy(66,27);
    return 0;
}

If there is any error, bug, or anything that can be modified please let me know.

Answer 1

A little disclaimer: I have not tested the code, so I'll assume it works as you intend and the game is playable.

A few insights on design and architecture

You say that this is a car game (a car-racing game I suppose), however, from looking at the code, there is nothing immediately obvious that corroborates that. You have only one class, CarG that does everything. I would expect to see many other classes for specific things, like a Car class to handle car-related logic, a Console class responsible for drawing the track/cars and textual UI, a Game class to tie everything together and keep track of scores, save games, etc... Not limited to these examples.

You should better separate concerns in your project. Have each class be a component of your game that does just one thing, but do this thing very well. Read about the Single Responsibility Principle.

Code review

Now focusing on the presented code:

1. Avoid abbreviated names that are unclear/hard to read. CarG would be much better as CarGame. lmove should be moveLeft (I'm guessing), same for rmove => moveRight. Some methods of your class are just impossible to guess. For instance cfun. That is a very unclear name. Ecar and Ecls are not much better... The point here is to give descriptive names. Don't worry about the name length, that's not a problem nowadays, we have the technology of auto-complete. Also, make sure to write them in a way that facilitates reading, meaning to use some notation to separate words in a name, like snake_case or camelCase. So don't write names like displayscore, use display_score or displayScore instead.

   Another point that fits in here is to use verbs or phrases that imply an action for function and method names. So displayScore() is good, but movement() not so much. Something like moveCars() would be better.

2. Still talking about naming, more care should should be taken when choosing your variables names. I know this is the hardest thing in programming, and that is specifically why we must strive for the best ;)

   Avoid single letter names, such as n and n1. sleep is also pretty vague. You have a lot of xs numbered as x1, x2, ... This can be very confusing. Take a step back and try to think of more descriptive names.

   For the record, there are a few cases where single letter variable names are fine. On such example are loop counters. i, j and k, for instance, are of a historical relevance, so they are basically standard, you don't have to worry about that.

3. This is a personal preference, but I prefer to place the public section of a class first in the header file. My rationale is that people using and reading my code will care much more about the public methods of class, so that's what they want to see first when reading the header file. Private/protected sections of a class are of interest to the programmer maintaining the code, so it makes sense that they don't need that much visibility.

4. Avoid using macros. They can be quite problematic if misused. They can also be very easily misused. C++ offers a lot of other (better) options, like typed constants, enums and template functions. Talking specifics, your CC macro looks quite useless. The code that is wraps is only used in one place, so it is not avoiding code duplication, but only obfuscating it.

5. There is a long discussion here about the issues of using namespace std;. In short, it is not well seen because it defeats the purpose of a namespace, which is allowing duplicate names to coexist without clashes.

6. COORD coord={0,0}; is a global variable for no good reason. It should be a local inside gotoxy().

7. Your code is filled with magic numbers. Try to move the ones that repeat themselves to named constants. The ones that are used in a single place, should at least be followed by a comment explaining why the given value was chosen.

8. Instead of using things like char(219), it would be better to type the character literal. If your code editor supports Unicode text, you should be able to. Otherwise, turn that into a named constant.

9. You have used the old rand() function and friends. You should also take a look at the new <random> library introduced by C++11.

10. Properly indent code (specially loops) to convey nesting. This:

    for(int k=-3;k<=4;k++)
    for(int j=0;j<=3;j++)
    

    Is a nice warm place for bugs to grow, forgive the pun. Nest the second loop under the first one. I also recommend always adding a pair of { } even for the single line statements, to facilitate future maintenance and expansion.

11. I would try to keep a more consistent code layout. Things like this:

    else if(keyp=='n' || keyp=='N')
    {   return 'q'; }
    else
    {   GameOver(); }
    

    Break the pattern and make the flow of logic harder to follow.

12. Use consistent spacing between arithmetical operators. Putting a space between operator and operand(s) greatly facilitates reading. Take this for instance:

    for(int j=1;j<=3;j+=2)
    

    Now with better spacing:

    for (int j = 1; j <= 3; j += 2)
    

---

This was a late answer, so hopefully your project is still going...

There are surely more details and aspects to consider, but I guess this is already long enough. If you apply some of these changes to your code, please feel free to post a new follow-up question and someone will be glad to review it.