[Snake] pierwszy samodzielny program obiektowy - prośba o ocenę.

Question

Witam wszystkich serdecznie. Napisałem swój pierwszy mały projekt orientowany obiektowo. Jest to popularna gra snake. Krótki opis gry:

• losuje nam położenie głowy węża na planszy
• losuje nam położenie muchy na planszy
• chodzimy w 4 kierunkach głową węża
• gdy zjemy muchę rośniemy
• gdy urośniemy możemy poruszać się tylko w 3 kierunkach bo wąż nie chodzi w tył
• gdy wejdziemy na ścianę lub zjemy sami siebie umieramy i kończymy grę

Grę napisałem na podstawie dwóch klas:

• Wall - obrazuje planszę gry
• Snake - obrazuje węża. Dodatkowo ta klasa dziedziczy w sposób protected po klasie Wall.

Chciałbym prosić o opinię i konstruktywną krytykę mojego kodu.

#include <iostream>
#include <cstdio>
#include <conio.h>
#include "Wall.h"
#include "Snake.h"

using namespace std;

int main()
{
    Wall wall;
    Snake snake;
    snake.go();
    wall.~Wall();
    cout << "KONIRC GRY" << endl;
    getch();
    return 0;
}

#ifndef WALL_H
#define WALL_H


class Wall
{
    public:
        Wall();
        ~Wall();


    protected:
        bool play;
        static const unsigned int SIZE = 20;     //map size
        char tab[SIZE][SIZE];           //map
        char vac;                       //empty space
        char brick;                     //edge of the map

        void draw();                    //draws current state on the map

    private:

};

   #endif // WALL_H

#include "Wall.h"
#include <iostream>
#include <stdlib.h>

using namespace std;

Wall::Wall()
{
    play = true;    //game started
    brick = '#';    //wall
    vac = ' ';      //empty space

    //first line
    for(unsigned int i=0; i<SIZE; i++)
        tab[0][i] = brick;

    //center empty
    for(unsigned int i=1; i<SIZE-1; i++)
        for(unsigned int j=1; j<SIZE-1; j++)
            tab[i][j] = vac;

    //first and last element of line(1 to SIZE-1)
    for(unsigned int i=1; i<SIZE-1; i++)
    {
        tab[i][0] = brick;
        tab[i][SIZE-1] = brick;
    }

    //last line
    for(unsigned int i=0; i<SIZE; i++)
        tab[SIZE-1][i] = brick;

}

Wall::~Wall()
{
    system("cls");
}

void Wall::draw()   //draws current state on the map
{
    for(unsigned int i=0; i<SIZE; i++)
    {
        for(unsigned int j=0; j<SIZE; j++)
                cout << tab[i][j];
        cout << endl;
    }
}

#ifndef SNAKE_H
#define SNAKE_H

#include "Wall.h"

class Snake :protected Wall
{
    public:
        void go();
        Snake();

    protected:
    private:

        char rep;   //reptile - snake
        char fly;
        unsigned int x;      //x-coordinate of the head snake
        unsigned int y;      //y-coordinate of the head snake
        static const unsigned int S = 99999;
        unsigned int tab_x[S];  //a table x-coordinate for storing previous positions of head snake
        unsigned int tab_y[S];  //a table y-coordinate for storing previous positions of head snake
        unsigned int length;    //length of snake
        unsigned int counter;   //counter of snake movements

        void random(char check, char Assign);
        void crawl(char dir);
        char select_direction_4();
        char select_direction_3(int one, int two, int there);
        void check();
        void get_fly();

};

    #endif // SNAKE_H

#include "Snake.h"
#include <ctime>
#include <cstdlib>
#include <cstdio>
#include <conio.h>

Snake::Snake()
{
    srand(time(NULL));

    length= 1 ;
    counter = 0;
    rep = 'o';
    fly = '*';
    int how_many = Wall::SIZE-2;
    x = (rand()%how_many)+1;   //random x-coordinate head of snake
    y = (rand()%how_many)+1;   //random y-coordinate head of snake
    Wall::tab[x][y] = rep;
    tab_x[0] = x;
    tab_y[0] = y;
    get_fly();                 //random fly

    Wall::draw();
}

void Snake::go()
{
    char dire;                  //stores the direction of the previous movement
    int one, two, there;        //help variables

    while(play and length == 1) //the possibility of moving in 4 directions
    {
            dire = select_direction_4();
            crawl(dire);        //make a move
    }

    while(play)                 //the possibility of moving in 3 directions, because the snake is not backward
    {
        switch(dire)            //change possible directions
        {
        case 72:    //up
        {
            one = 72;
            two = 75;
            there = 77;
            break;
        }
        case 80:    //down
        {
            one = 80;
            two = 75;
            there = 77;
            break;
        }
        case 75:    //up
        {
            one = 72;
            two = 75;
            there = 80;
            break;
        }
        case 77:    //up
        {
            one = 72;
            two = 80;
            there = 77;
            break;
        }
        }
        dire = select_direction_3(one, two, there);
        crawl(dire);            //make a move
    }

}
void Snake::crawl(char dir)
{
   switch(dir)
   {
       case 72: //up
       {
           x--;
           break;
       }
       case 80: //down
       {
           x++;
           break;
       }
       case 75: //left
       {
           y--;
           break;
       }
       case 77: //rigth
       {
           y++;
           break;
       }
   }

   check();         //check if terminate a game
   if(!play) return;

   counter++;
   tab_x[counter] = x;
   tab_y[counter] = y;

   bool random_fly = false;
   if(Wall::tab[x][y] == fly)   //if you eat a fly
   {
       length++;
       random_fly = true;
   }
   Wall::tab[x][y] = rep;

   //The following lines serve to clean the end of snake
   int index = counter-length;
   int xx = tab_x[index];
   int yy = tab_y[index];
   Wall::tab[xx][yy] = Wall::vac;
   /////////////////////////////////////////////////////

   if(random_fly)       //if you eat a fly
       get_fly();

   system("cls");
   Wall::draw();

}

char Snake::select_direction_4()
{
    char dir;
    do
    {
        dir = getch();
    }
    while(dir != 72 and dir != 80 and dir != 75 and dir != 77);
    return dir;
}

char Snake::select_direction_3(int one, int two, int there)
{
    char dir;
    do
    {
        dir = getch();
    }
    while(dir != one and dir != two and dir != there);
    return dir;
}

void Snake::check() //check if terminate a game
{
    if(tab[x][y] == brick or tab[x][y] == rep)
        play=false;
}

void Snake::get_fly()   //random a fly
{
    srand(time(NULL));
    int how_many = Wall::SIZE-2;
    int x_fly, y_fly;

    do
    {
        x_fly = (rand()%how_many)+1;
        y_fly = (rand()%how_many)+1;
    }
    while(Wall::tab[x_fly][y_fly] != vac);

    Wall::tab[x_fly][y_fly] = fly;
}

 

Comments

Nie uzywaj "using namespace std" ani zadnego innego, w przyszlosci moze to spowodowac konflikt nazw

---

QizmoPL:

#include <iostream>
using namespace std;

int main() {
  int cout = 3;
  std::cout << cout;//działa, choć może być mylące
}

Answer 1

Wall wall;
Snake snake;
snake.go();
wall.~Wall();
cout << "KONIRC GRY" << endl;
getch();
return 0;

Przy wywołaniu destruktora obiekt się nie niszczy. Traktować go można tutaj jak zwykłą funkcję. Destruktor wywoła się drugi raz przy (niepotrzebnym, bo to main, mógłbyś od razu dać klamrę) return. Jeśli chcesz, by się wywołał jednokrotnie (raczej powinien), proponuję:

 

{
    Wall wall;//konstruktor Wall
    Snake snake;//konstruktor Wall dla węża (dziedziczenie...), potem konstruktor Snake
    snake.go();
//tutaj destruktor Snake dla węża
//następnie destruktor Wall dla węża (bo dziedziczy, wtf)
//później destruktor Wall dla ściany
}
cout << "KONIRC GRY" << endl;//och, literówka
getch();

Ujawnia się kolejny problem: Snake dziedziczy z Wall, co można by odczytać jako: "Wąż jest ścianą". Zależności między tymi klasami są coś "nie teges". Obecnie, w rozgrywce są dwie ściany: jedna to wall, druga to odziedziczona część węża.

Nie pisałbym żadnego specjalnego destruktora dla żadnej z tych klas. system("cls") możesz gdzieś przenieść.

srand(time(NULL)) powinno wywołać się raz, początek main to dobre miejsce na to.
private jest lepsze od protected.

PS: To tylko parę cech prawdopodobnie do poprawy. Nie traktuj "listy" jako kompletnej.

Comments

Dzięki za rady, zastosowałem się już to Twoich uwag w programie. Nadal tylko nie wiem co jest źle z zależnościami między tymi klasami, mógłbyś to rozjaśnić i pokazać jakiś przykład? :D

---

Nie widzisz nic podejrzanego w tym, że Snake jest ścianą? XD To po prostu nasuwa podejrzenia, że coś jest nie tak.

---

Mógłbyś zaproponować alternatywną wersję(chodzi mi o prosty opis)? Też użył byś dwóch klas w projekcie?

---

Widziałbym to jakoś tak:
• brak dziedziczenia, chyba że tylko funkcji głównie virtual [...] = 0
• składowe protected zmienione na private
• wąż nie odnosi się do siebie-jako-ściany (Wall::właściwość), lecz do zewnętrznej ściany: metody, które tego potrzebują, przyjmują wskaźnik/referencję do ściany jako argument
 

---

O co chodzi z tym virtual[...]=0? Wiem, że to funkcja wirtualna ale nie wiem jak zinterpretować to w tym kontekście. Jak to ma działać?

Zakładając że klasa Snake nie będzie dziedziczyła z klasy Wall to w jaki sposób odnosić się do składników private Wall z poziomu klasy Snake? Coś mi świta, że można zrobić publiczną funkcję w klasie Wall która będzie zwracała poszczególne dane. Czy to dobry sposób?

---

Popieram pomysł z tą funkcją od danych. Jeśli jakaś czynność dotyczy przede wszystkim właśnie ściany, zamiast pobierania danych i wykonywania na nich operacji lepiej będzie kazać ścianie, by sama się zarządziła. Odnośnie składni =0 poszukaj o funkcjach czysto wirtualnych.