Game classes interaction

Question

Since I'm new to coding, can you give me some advise on game class interactions? The game does work, but my code looks ugly and I think it can be improved a lot.

What I'm mostly interested in:

1. class1.setvalue(class2.returnvalue()) is this how it is supposed to be done?
2. Should I derive my objects from some virtual base?
3. If I would like to add another enemy (or food) class object at some point of the game, how should it be done?
4. The use of int k to make enemy move in only half of cycles.

Don't go rough on console things. No one makes console games anyway.

#include "stdafx.h"
#include <iostream>
#include <conio.h>
#include <vector>
#include <windows.h>
using namespace std; //know its a bad practise

struct coord
{
    int x;
    int y;
    char mark;
};

class karta //map
{
public:
    karta(int a, int b )
    {
        mx = a; my = b;
        for (i = 0; i < mx; i++)
        {
            vector<char> clm; 
            for (j = 0; j < my; j++)
            {
                clm.push_back(' '); 
            }
            mapa.push_back(clm); 
        }
    }
void generatemap()  
    {
        for (i = 1; i < mx; i++)
            for (j = 1; j < my; j++)
                mapa[i][j] = ' ';

        for (i = 1; i <= my; i++)
        {
            mapa[0][i-1] = 177;
            mapa[mx-1][i-1] = 177;
        }
        for (i = 1; i <= mx; i++)
        {
            mapa[i-1][0] = 177;
            mapa[i-1][my-1] = 177;
        }
    }
void generateobjects() 
{
    mapa[food.x][food.y] = food.mark;
    mapa[hero.x][hero.y] = hero.mark;
    mapa[enemyc.x][enemyc.y] = enemyc.mark;
}
void printemap() 
    {
        for (j = 0; j < my; j++)
        {
            for (i = 0; i < mx; i++)
            cout << mapa[i][j];
            cout << endl;
        }
    }

void updateobjectdata(coord k)
{
    hero = k;
}
void setfoodc(coord k) 
{
    food = k;
}
void updatenemy(coord k)
{
    enemyc = k;
}

int pass_x() 
{
    return mx;
}
int pass_y() 
{
    return my;
}

private:
    int i,j;
    int mx;
    int my;
    vector< vector<char> > mapa; 
    coord hero,food,enemyc;
};

class game
{
public:
    game() 
    {
        running = true;
        score = 0;
        flag = true; 
    }

void setwalls(int x, int y)
    {
        wallx = x-1; wally = y-1;
    }
void sethero(coord k)
    {
        heroc = k;
    }
void setfood(coord k)
    {
        foodc = k;
        flag = false;
    }
void setenemy(coord k)
    {
        enemyc = k;
    }

bool ifrunning()
    {
        if (running)
            return true;
    }

void checkwall()
    {
        if (wallx == heroc.x || wally == heroc.y || heroc.x == 0 || heroc.y == 0)
        {
            running = false;
        }

    }
void checkenemy()
    {
        if (heroc.x == enemyc.x && heroc.y == enemyc.y)
        {
            running = false;
        }
    }

int enemymovement()
    {
        if (heroc.x > enemyc.x)
            if (heroc.y > enemyc.y)
                if (rand() % 2 == 1)
                    return 1;
                else
                    return 3;
            else
                if (heroc.y == enemyc.y)
                    return 1;
                else
                    if (rand() % 2 == 1)
                        return 1;
                    else
                        return 4;
        else
            if (heroc.y > enemyc.y)
                if (heroc.x == enemyc.x)
                    return 3;
                else
                    if (rand() % 2 == 1)
                        return 2;
                    else
                        return 3;
            else
                if (heroc.x == enemyc.x)
                    return 4;
                else
                    if (heroc.y == enemyc.y)
                        return 2;
                    else
                        if (rand() % 2 == 1)
                            return 2;
                        else
                            return 4;

    }

void checkfood()
    {
        if (heroc.x == foodc.x && heroc.y == foodc.y)
        {
            score += 5;
            flag = true;
        }
    }

bool flagstatus()
    {
        return flag;
    }

void gameover() 
    {
        cout << "Game over! Your score is :" << score;
    }

private:
    bool running,flag;
    int wallx, wally, score;
    coord heroc,foodc,enemyc;
};

class enemy
{
public:
    enemy(int a, int b)
    {
        limx = a - 2; limy = b - 2;
        obj.mark = 'G';
    }
void gencoord()
    {
        obj.x = rand() % limx + 1;
        obj.y = rand() % limy + 1;
    }
void move(int a)
    {
        switch (a)
        {
        case 1:
            obj.x++;
            break;
        case 2:
            obj.x--;
            break;
        case 3:
            obj.y++;
            break;
        case 4:
            obj.y--;
            break;
        }
    }

coord givecord()
    {
        return obj;
    }
private:
    coord obj;
    int limx, limy;
};

class object //hero
{
public:
    object()
    {
        obj.x = 6;
        obj.y = 6;
        obj.mark = '$';
    }

void move(char key)
    {
        switch (key)
        {
        case 'w':
            obj.y--; // inversed!
            break;
        case 'a':
            obj.x--;
            break;
        case 'd':
            obj.x++;
            break;
        case 's':
            obj.y++; // inversed!
            break;
        }
    }

coord givecord()
{
    return obj;
}

private:
    coord obj;
};

class food
{
public:
    food(int a, int b) 
    {
        limx = a-2; limy = b-2;
        obj.mark = '5';
    }

void gencoord()
    {
        obj.x = rand() % limx +1;
        obj.y = rand() % limy +1;
    }

coord givecord()
    {
        return obj;
    }

private:
    coord obj;
    int limx, limy;
};

int main()
{
    karta mapka(30,15); //size depends on your console
    object hero;
    game gamz;
    food eda(mapka.pass_x(), mapka.pass_y());
    enemy vrag(mapka.pass_x(), mapka.pass_y());
    vrag.gencoord();
    int k = 0;

    gamz.setwalls(mapka.pass_x(), mapka.pass_y());
    gamz.sethero(hero.givecord());
    gamz.setenemy(vrag.givecord());

    mapka.generatemap();
    mapka.updateobjectdata(hero.givecord());
    mapka.updatenemy(vrag.givecord());
    mapka.generateobjects();
    mapka.printemap();

    Sleep(2000);
    _getch();

    while (gamz.ifrunning() == true)
    {
        Sleep(110);
        if (_kbhit()) 
        {
            hero.move(_getch());
        }
        gamz.sethero(hero.givecord());
        gamz.checkenemy();
        gamz.checkwall();
        gamz.checkfood();
        if (gamz.flagstatus() == true)
        {
            eda.gencoord();
            gamz.setfood(eda.givecord());
            mapka.setfoodc(eda.givecord());
        }

        if (k%2 == 1)
            vrag.move(gamz.enemymovement());
        k++;
        gamz.setenemy(vrag.givecord());
        gamz.checkenemy();

        system("cls");
        mapka.updateobjectdata(hero.givecord());
        mapka.updatenemy(vrag.givecord());
        mapka.generatemap();
        mapka.generateobjects();
        mapka.printemap();
    }
    gamz.gameover();

    Sleep(5000);
    _getch();

    return 0;
}

$ - you; 5 - food; G - enemy; WASD - move. Game language must be English

Answer 1

struct coord
{
    int x;
    int y;
    char mark;
};

OOP is not only about how you encapsulate data and functions into objects - it's also about defining how objects interact with each other.

Let's pick the coord object as an example. As you use it, it's just a container, but not exactly object oriented. This line actually characterizes it well:

heroc.x == foodc.x && heroc.y == foodc.y

Despite having two coord instances which are in a trivial relation to each other, you couldn't just compare them, because you didn't implement cood::operator==(). Instead you needed to copy the corresponding logic into every single site attempting to compare two coord instances.

---

mapa[0][i-1] = 177;

What is 177? OK, I know from the context it's an extended ASCII code. But without looking it up in a table, I don't know which one.

This makes 177 a magic number.

The problem with that is, you reused it multiple times, and every single time in numeral form. If I wanted to switch out the ASCII symbol used for the border, I would need to check all occurrences of this number. But even if I did - I could not tell whether it had the same semantic.

Use constants instead of magic numbers:

enum class symbol {
    empty = ' ',
    player = '$',
    food = '5',
    enemy = 'G',
    border = 177
}

// Sample usage
map[i][j] = symbol::empty;

Now it's both clear what each symbol means when occurring in the code base, and there is now only a single location which needs to be changed when you want to change the symbols.

---

if (rand() % 2 == 1)
    return 1;
else
    return 3;

This case of magic numbers is even worse, because this time I can't even deduce from the context what these numbers are supposed to mean.

However, this time the solution is not to uses constants, there is a better solution. Remember how you didn't complete the work on coord?

It's time to implement another operator, coord::operator+(). With that operator implemented, you can simply return an coord instance representing the offset the enemy is supposed to move, and add it to the enemies current location. Absolutely no magic number needed this time.

---

game gamz;
food eda;
enemy vrag;

Mixing languages is typically a horrible idea. A sane guideline, stick to the language predominant in the framework you are using. In the case of C++ and the STL, that's exclusively English.

This is not only because other developers most likely won't be able to understand why you named something in a specific way - it's also ambiguous. For every possible variable, you want to have a naming scheme which allows only one sensible name for that variable so that you never have to guess how you named something. Having two languages to choose from, makes this already a futile attempt.

---

generatemap
generateobjects
printemap
setfoodc
...

All of these names are actually rather difficult to read. Why? Because chaining words without gaps doesn't work all to well.

You should make use of a naming convention which clearly separates the individual words forming an identifier. The two usual conventions for that are cammelCase and snake_case, with the former one being commonly used in C# or Java, and the latter one being typical in C and C++.

---

Sleep(2000);
_getch();

And now we halt the program for 2 seconds, and expect the user to know that he should press a key to continue.

There is the instructions missing. Either that, or that delay is entirely pointless.

Sleep(5000);
_getch();

And because it was so nice, we do it again, but this time we prevent the user from terminating the application.

---

if (_kbhit()) 
{
    hero.move(_getch());
}

Nice try, but this processes only one character per frame, without discarding any other inputs pending in the input buffer. So if the user smashed the keyboard during the aforementioned sleep in expectation of a reaction, all these unprocessed inputs are now being executed one by one.

---

_kbhit();
_getch();
system("cls");

All 3 of these only work as expected (or at all!) on Windows. Cross platform input handling and interaction with the terminal is rather difficult.

Answer 2

So @Ext3h did already cover a lot. here some more thoughts.

1. If you know its bad dont do it. --> looking at namespace std;

2. For gods sake use braces. You are in your 5th level of nesting without a single brace. That is simply a bug in waiting. As a rule of thumb, whenever you nest something use braces. It is not like the additional line is hurting you.

3. Similar to the above declare each variable on its own line. That way it is much easier to read.

4. Use separate files for each class you have. That makes it much easier to follow your code.

5. Your main function is loaded with stuff that should be part of the class initialization.

   gamz.setwalls(mapka.pass_x(), mapka.pass_y());
   gamz.sethero(hero.givecord());
   gamz.setenemy(vrag.givecord());
   
   mapka.generatemap();
   mapka.updateobjectdata(hero.givecord());
   mapka.updatenemy(vrag.givecord());
   mapka.generateobjects();
   mapka.printemap();
   

Try to think what your class need to be fully defined and put that in the constructor. As a rule of thumb, whenever you need to do something outside of a class object before you can safely use it, something is wrong.