obiektowość nauka

Odpowiedz Nowy wątek
2019-07-03 15:43

Rejestracja: 11 miesięcy temu

Ostatnio: 10 miesięcy temu

0

Cześć napisałem kółko i krzyżyk obiektowo tak jak umiem bez przypominania sobie czego wcześniej się uczyłem bez szukania w necie czegokolwiek na temat kółka i krzyżyk. Proszę o porady co zmienić, czego się nauczyć co ogarnąć lub czy robić dalej takie gry jak snake, pacman itd... w taki sam sposób jak zrobiłem to. Moim celem jest jak wiadomo nauka programowania obiektowego ale również nauka pisania 'większych' projektów ;) Czekam konstruktywne opinie

#include <iostream>
#include <ctime>
#include <cstdlib>

using namespace std;

class Game
{
private:
    char arrayGame[9];
    bool gameStatus;
    int player;
    char sign;
    bool draw;

public:
    Game();
    ~Game();

    void drawBoard();
    void choosePlayer();
    void makeMove();
    void winConditions();
    bool gameOver();
    bool drawInGame();
};

int main()
{
    cout << " Kolko i krzyzyk! " << endl;

    Game ticTacToe;

    for (;;)
    {
        if (ticTacToe.gameOver() == false)
        {
            system("CLS");
            ticTacToe.choosePlayer();    // losuje kto będzie się ruszał pierwszy i przypisuje znak
            ticTacToe.drawBoard();       // rysuje tablice
            ticTacToe.makeMove();        // gracz wybiera jakie pole chce zająć
            ticTacToe.winConditions();   // sprawdzane są warunki zwycięstwa (jeśli któryś z graczy wygrał/remis-koniec gry, w przeciwnym wypadku następny obieg pętli)
        }
        else
            break;
    }

    system("CLS");
    ticTacToe.drawBoard();
    if (ticTacToe.drawInGame() == true)
    {
        cout << " Remis! " << endl;
    }
}

Game::Game()
{
    gameStatus = true;
    player = 0;
    sign = '-';
    draw = false;
    for (int i = 0; i < 9; i++)
    {
        arrayGame[i] = i+49;
    }
}

Game::~Game()
{
    // Nic nie alokuje dynamicznie, więc nie potrzebuję destruktora.
}

void Game::drawBoard()
{
    if(gameStatus==true)
        cout << "ruch gracza " << player << endl;
    else
    {
        cout << " Wygral gracz: " << player << ".   Gratulacje!" << endl;
    }
    cout << endl;
    cout << "=============" << endl;
    cout << "| " << arrayGame[0] << " | " << arrayGame[1] << " | " << arrayGame[2] << " |" << endl;
    cout << "|-----------|" << endl;
    cout << "| " << arrayGame[3] << " | " << arrayGame[4] << " | " << arrayGame[5] << " |" << endl;
    cout << "|-----------|" << endl;
    cout << "| " << arrayGame[6] << " | " << arrayGame[7] << " | " << arrayGame[8] << " |" << endl;
    cout << "=============" << endl;
}

void Game::choosePlayer()
{
    if (player == 0)
    {
        srand((int)time(NULL));
        player = (rand() % 2) + 1;      // 1-gracz pierwszy    2-gracz drugi
    }

    player = (player % 2) + 1;          // 'skakanie' pomiędzy pierwszym a drugim graczem 1,2,1,2,1...
    if (player == 1)
    {
        sign = 'X';
    }
    else if (player == 2)
    {
        sign = 'O';
    }
}

void Game::makeMove()
{
    int position = 0;
    cout << " Wybierz pole ktore chcesz zajac" << endl;
    while (position < 1 || position > 9 || position == 0)
    {
        cout << " Wprowadz numer od 1 do 9" << endl;
        cin >> position;
        if (arrayGame[position - 1] >= 1 || arrayGame[position - 1] <= 9)
        {
            arrayGame[position - 1] = sign;
        }
        else
        {
            system("CLS");
            cout << " To pole jest juz zajete! " << endl;
            drawBoard();
            position = 0;
        }
    }
}

void Game::winConditions()
{
    // warunki zwycięstwa poziomo
    for (int i = 0; i <= 6; i += 3)
        if ((arrayGame[i] == 'X' && arrayGame[i + 1] == 'X' && arrayGame[i + 2] == 'X' ) || (arrayGame[i] == 'O' && arrayGame[i + 1] == 'O' && arrayGame[i + 2] == 'O'))
        {
            gameStatus = false;
            break;
        }

    // warunki zwycięstwa pionowo
    for (int i = 0; i <= 2; i++)
        if ((arrayGame[i] == 'X' && arrayGame[i + 3] == 'X' && arrayGame[i + 6] == 'X') || (arrayGame[i] == 'O' && arrayGame[i + 3] == 'O' && arrayGame[i + 6] == 'O'))
        {
            gameStatus = false;
            break;
        }

    // warunki zwycięstwa na krzyż
    if ((arrayGame[0] == 'X' && arrayGame[4] == 'X' && arrayGame[8] == 'X') || (arrayGame[0] == 'O' && arrayGame[4] == 'O' && arrayGame[8] == 'O'))
    {
        gameStatus = false;
    }
    if ((arrayGame[2] == 'X' && arrayGame[4] == 'X' && arrayGame[6] == 'X') || (arrayGame[2] == 'O' && arrayGame[4] == 'O' && arrayGame[6] == 'O'))
    {
        gameStatus = false;
    }

    // warunki remisu
    bool draw = false;
    for (int i = 0; i < 9; i++)
    {
        if ((arrayGame[i] == 'X' || arrayGame[i] == 'O') && (i == 8))
        {
            gameStatus = false;
            draw = true;
        }
        else
            break;
    }
}

bool Game::drawInGame()
{
    if (draw == true)
        return true;            // remis
    else
        false;
}

bool Game::gameOver()
{
    if (gameStatus == false)
        return true;            // gra zakończona

    else
        return false;
}
edytowany 1x, ostatnio: kacperr, 2019-07-03 15:56

Pozostało 580 znaków

2019-07-03 16:25

Rejestracja: 5 lat temu

Ostatnio: 2 tygodnie temu

2

Łatwiej, krócej:

bool Game::drawInGame()
{
    return draw;
}

bool Game::gameOver()
{
    return !gameStatus;
}



Dobrą praktyką jest wstawianie klamerek nawet jak nie są wymagane

for (int i = 0; i <= 6; i += 3)
{
        if ((arrayGame[i] == 'X' && arrayGame
if (gameStatus)
{
    cout << "ruch gracza " << player << endl;
}

Pozwala to uniknąć błędów potem, gdy się doda instrukcje ale się zapomni o klamrach.


Game::~Game()
{
    // Nic nie alokuje dynamicznie, wiêc nie potrzebujê destruktora.
}

Jak nie potrzebujesz własnego destruktora, to po prostu nie deklaruj/definiuj. Są przypadki, kiedy potrzebujesz domyślnego destruktora, a nie jest generowany, to wystarczy

~Game() = default;


Game::Game()
{
    gameStatus = true;
    player = 0;
    sign = '-';
    draw = false;

Zamiast przypisywać, lepiej użyć listy inicjalizacyjnej

Game::Game()
    : gameStatus(true)
    , player(0)
    , sign('-')
    , draw(false)
{
Trzeba pamiętać że jawnie zadeklarowany domyślny destruktor ~Game() = default; spowoduje że kompilator nie wygeneruje domyślnego konstruktora przenoszącego Game(Game &&) oraz domyślnego operatora przenoszącego Game & operator=(Game &&). Dlatego jeżeli nie chcemy używać swoich własnych implementacji powyższych konstrukcji to lepiej zastosować się do zasady rule of zero. - TomaszLiMoon 2019-07-04 08:12

Pozostało 580 znaków

2019-07-03 16:34

Rejestracja: 11 miesięcy temu

Ostatnio: 10 miesięcy temu

0

bardzo dziękuje za poprawę. Myślisz, że ten projekt zamknąć i zacząć robić snake? czy raczej wrócić do nauki obiektowości z książki przed następnym projektem?

Pozostało 580 znaków

2019-07-03 16:39

Rejestracja: 1 rok temu

Ostatnio: 18 sekund temu

1
kacperr napisał(a):

bardzo dziękuje za poprawę. Myślisz, że ten projekt zamknąć i zacząć robić snake? czy raczej wrócić do nauki obiektowości z książki przed następnym projektem?

Uwagi kolegów co do kodu są OK. Ale projekt jak i porady daleki/e są od obiektowości w głębokim sensie.

Popieram. Czym ten projekt różniłby się od takiego, w którym wywalasz klasę Game i wszystkie jej wnętrzności wrzucasz po prostu jako zwykłe funkcje/zmienne globalne ? - Bartłomiej Golenko 2019-07-03 17:00

Pozostało 580 znaków

2019-07-03 17:12

Rejestracja: 5 lat temu

Ostatnio: 14 minut temu

1

Co do obiektowości:
Jedna klasa przechowująca wszystko to niedobre podejście. Podziel to - niech Game przechowuje/korzysta z mniejszych klas takich jak: Player, Board.
Dla urozmaicenia możesz spróbować swoich sił z MVC - niech gra ma widok (drawBoard(), drawInGame()), kontroler (na przykład MakeMove()) i model, który będzie przechowywał informacje o grze modyfikowane przez kontroler.


Co do samego kodu:
Uwagi @twonek są słuszne, ja przyczepię się jeszcze do nazewnictwa:

  • winConditions() <- nie mam pojęcia co robi ta metoda. Wyświetla warunki? Sprawdza je? Czyta z pliku?
  • gameOver() <- nazwa zwracająca taki warunek powinna zawierać is, tak aby odpowiadała na pytanie. Obecnie ma się wrażenie, że metoda ta obsługuje moment zakończenia gry, a nie sprawdza czy taki wystąpił.
  • gameStatus <- zdaję sobie sprawę, że to pole prywatne, ale skoro przechowuje bool, a nie enum to nazwałbym to tak, by można było temu zaprzeczyć/potwierdzić: isGameOver, gameFinished

Pozostało 580 znaków

Odpowiedz

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0