gra w kółko i krzyżyk

0

Dawno nic nie pisałem w C++, więc w ramach odświeżenia składni napisałem sobie grę w kółko i krzyżyk (wersja konsolowa).
Przyznam, że przyzwyczajenia z języków skryptowych trochę przeszkadzały ;)

#include <iostream>
#include <stdlib.h>
#include <conio.h>
#include <stdio.h>

using namespace std;

void rysuj(char plansza[])
{
    cout << "-------------------" << endl;
    cout << "|  " << plansza[0] << "  |  " << plansza[1] << "  |  " << plansza[2] << "  |" << endl;
    cout << "-------------------" << endl;
    cout << "|  " << plansza[3] << "  |  " << plansza[4] << "  |  " << plansza[5] << "  |" << endl;
    cout << "-------------------" << endl;
    cout << "|  " << plansza[6] << "  |  " << plansza[7] << "  |  " << plansza[8] << "  |" << endl;
    cout << "-------------------" << endl << endl;
}

void ruch(char gracz, char plansza[])
{
    char pole;
    int index;
    cout << "Ruch gracza " << gracz << endl;
    do
    {
        cout << "Podaj numer pola: ";
        cin >> pole;
        index = pole - 48;
    }
    while ((pole < 49) || (pole > 57) || plansza[index-1] != ' ');
    plansza[index-1] = (gracz == 'X') ? 'X' : 'O';
}

void wynik(int tura, char gracz, bool & wygrana, char plansza[])
{
    if (
       ((plansza[0] == plansza[1]) && (plansza[1] == plansza[2]) && (plansza[0] != ' ')) ||
       ((plansza[3] == plansza[4]) && (plansza[4] == plansza[5]) && (plansza[3] != ' ')) ||
       ((plansza[6] == plansza[7]) && (plansza[7] == plansza[8]) && (plansza[6] != ' ')) ||
       ((plansza[0] == plansza[3]) && (plansza[3] == plansza[6]) && (plansza[0] != ' ')) ||
       ((plansza[1] == plansza[4]) && (plansza[4] == plansza[7]) && (plansza[1] != ' ')) ||
       ((plansza[2] == plansza[5]) && (plansza[5] == plansza[8]) && (plansza[2] != ' ')) ||
       ((plansza[0] == plansza[4]) && (plansza[4] == plansza[8]) && (plansza[0] != ' ')) ||
       ((plansza[2] == plansza[4]) && (plansza[4] == plansza[6]) && (plansza[2] != ' '))
       )
    {
        cout << "Wygrywa gracz " << gracz << endl;
        wygrana = true;
    }
    else if (tura == 9)
    {
        cout << "Koniec gry, remis. " << endl;
        wygrana = true;
    }
}

int main()
{
    char plansza[9] = {' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '};
    char gracz = ' ';
    int tura = 0;
    bool wygrana = false;

    while (!wygrana)
    {
        rysuj(plansza);
        wynik(tura, gracz, wygrana, plansza);
        if (!wygrana)
        {
            gracz = (tura % 2 == 0) ? 'X' : 'Y';
            ruch(gracz, plansza);
            tura++;
        }
        getchar();
        system("cls");
    }
    return 0;
}



1

Pierwsza sprawa - jest to raczej kwestia estetyki i przyjętych konwencji, ale staramy się używać angielskich nazw funkcji i zmiennych. Jak piszesz sam, a zwłaszcza gdy się uczysz, to jeszcze nie ma tragedii z polskimi, ale chyba lepiej od początku uczyć się dobrych zwyczajów. Dobrze przynajmniej, że jesteś konsekwentny i trzymasz się jednej konwencji (z wyjątkiem index, który za bardzo na polskie słowo nie wygląda ;) ). Bo najgorzej jak ktoś tworzy potworki w stylu PokazResults albo coś podobnego, na co od samego patrzenia można dostać raka oczu :D

if (
       ((plansza[0] == plansza[1]) && (plansza[1] == plansza[2]) && (plansza[0] != ' ')) ||
       ((plansza[3] == plansza[4]) && (plansza[4] == plansza[5]) && (plansza[3] != ' ')) ||
       ((plansza[6] == plansza[7]) && (plansza[7] == plansza[8]) && (plansza[6] != ' ')) ||
       ((plansza[0] == plansza[3]) && (plansza[3] == plansza[6]) && (plansza[0] != ' ')) ||
       ((plansza[1] == plansza[4]) && (plansza[4] == plansza[7]) && (plansza[1] != ' ')) ||
       ((plansza[2] == plansza[5]) && (plansza[5] == plansza[8]) && (plansza[2] != ' ')) ||
       ((plansza[0] == plansza[4]) && (plansza[4] == plansza[8]) && (plansza[0] != ' ')) ||
       ((plansza[2] == plansza[4]) && (plansza[4] == plansza[6]) && (plansza[2] != ' '))
       )
    {

Co do tego fragmentu mam dwa zastrzeżenia:

  1. taki 8-liniowy zagnieżdżony if, w którym w każdej linii masz 3 warunki to jakaś masakra. Wprawdzie zakładam, że robi co ma robić, ale wygląda paskudnie. Jeśli masz pomysł jak można to skrócić to może zrób to, jeśli nie, to pokombinuj. Ale - niezależnie od tego, czy coś wymyślisz czy nie - dobrze jest skorzystać z porady numer 2
  2. takie rzeczy powinno się wywalić do osobnej funkcji. Wtedy (pomijając czy bardziej czy mniej paskudny ten if```` będzie) czytając kod nie muszę patrzeć na te wszystkie warunki - bo są one przede mną ukryte. A ja w miejscu gdzie aktualnie znajduje się ten klocek z if'amam jedynie odwołanie do funkcji na zasadzieif ukladWygrywajacyi wiem, co to robi - sprawdza, czy na planszy jest układ wygrywjący i jeśli tak, to wykonuje co jest zaif```. A sam mechanizm sprawdzenia mnie w tej chwili nie interesuje, skupiam się jedynie na logice - interesuje mnie tylko to, czy w danym momencie mamy 3 znaczki w linii.
 char plansza[9] = {' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '};

Zapewne widziałeś plansze do tej gry. Nie rzuca Ci się w oczy, że jest to tabelka o rozmiarze 3x3? Oczywiście - można to trzymać tak, jak zrobiłeś, ale czy nie lepiej zrobić z tego tablicę dwuwymiarową, której układ będzie odzwierciedlać realny układ gry? Coś na zasadzie char plansza[3][3].

Skoro się uczysz, to za chwilę pojawi się bardzo popularne hasło, czyli OOP (programowanie obiektowe). Możesz spróbować przerobić to na obiektówkę. Aż się prosi, żeby plansza i gra były obiektami. Przy okazji - zobacz, ile rzeczy pojawi się do rozważenia - czy plansza i gra mają być osobnymi klasami? Może jedna ma być elementem składowym drugiej? Która klasa za co będzie odpowiadać - np. wykonanie ruchu to zakres odpowiedzialności planszy czy kontrolera gry? Która klasa ma rysować grę na ekranie?

EDIT
W komentarzu @kosmonauta80 napisał Co jest prościej okodować: char plansza[3][3] czy char plansza[9]? ;) Chodzi o odczyt i zapis. .
Pytanie - co Ty masz na myśli/co rozumiesz pod hasłem "prościej". OK, robiąc tablicę 2D masz jeden nawias i jedną cyfrę więcej do napisania. Ale kod przede wszystkim ma być czytelny.
Zastanów się, czy jak masz tablica[5] to od razu wiesz, o które pole chodzi? Czy przypadkiem zapis tablica[2][1] nie jest bardziej czytelny?

1

Będę szedł sukcesywnie, edytując ten post, część pierwsza póki co ;)

Nazewnictwo zmiennych po polsku to najmniejszy problem.

Lecim top-down.

#include <iostream>
#include <cstdlib> //<stdlib.h>
//#include <conio.h> w ogóle nie jest potrzebne 
#include <cstdio> //<stdio.h>

using namespace std; //jak to jeden plik to ujdzie ale ogólnie nie polecam

void rysuj(char plansza[])
Nawala tu kilka rzeczy:
a. const correctness. czemu nie const char[] albo const char* const.
b. jak bierzesz tablicę o znanej wartości, łap ją przez referencję, wtedy masz sprawdzanie rozmiaru za darmo void rysuj(const char (&plansza)[9])
c. Zamknij planszę w klasie i przeciąż dla niej ostream& operator<<(std::ostream, const Plansza&)
W samym wydruku to w sumie nie bardzo jest co popsuć, ale jedna rzecz: nie używaj endl, tylko \n, to pierwsze robi niepotrzebny flush.

EDIT 1:

void ruch(char gracz, char plansza[])
Znowu to samo co wcześniej jeśli chodzi o tablicę, użyj referencji. Albo osobnej klasy na planszę.
Rozdzieliłbym input i faktyczny ruch.
Co mógłbyś zrobić nieco na modłę funkcyjną: ruch zwraca nową planszę jeśli jest prawidłowy a nie modyfikuje wejścia. To by było łatwiejsze do testowania. Ponadto: zarówno gracz jak i pola na planszy mogłyby być jakimś enumem, a nawet jeden podzbiorem drugiego, zwróć uwagę, że plansza to X-Y-pusto, a gracze to X i Y. Twoje struktury danych tego nie oddają.
Docelowo widziałbym tę funkcję +- tak: std::optional<Board> boardAfterMove(const Board& board, const Player& player, std::size_t field). Może to być także metoda klasy board...

BTW, obadałem include'y dokładniej i wychodzi że tam tylko iostream jest realnie potrzebny, jeśli koniecznie chcesz mieć getchara to jeszcze cstdio (ale po co Ci ten getchar i mieszanie API? żeby "zjeść" enter?), cstdlib to nie wiem po co?

Ogólna uwaga: mamy 2021 rok, zaraz 2022. Polecałbym używać nowego standardu C++, co najmniej 11, a najlepiej to 14, 17 a może i 20, tam jest masa przydatnych ficzerów vide ten optional, lambdy, std::array i podobne i naprawdę mogłoby to wiele w Twoim kodzie uprościć.

1

Pomysł by zrobić wersję OOP podoba mi się. Niebawem przedstawię taką wersję uwzględniając także powyższe uwagi.

1

A nie jest tak, że jak przekazuję do funkcji tablicę, to przekazuję jej referencję?

int tab[3] {1,2,3};
const int ctab{5,7,8}; 
void f(int x[]); //tablica zamienia się na wskaźnik do jej początku ergo:
void g(int* x); //efektywnie to samo co wyżej
f(tab) // int* p= &tab[0]; f(p);
g(tab) // to samo. Do tego zawartość tablicy może być modyfikowana (int* -> wskaźnik na mutowalny int)
g(ctab); //compile error, ctab is const
f(ctab). //jw;
void f(const int x[]); //tablica zamienia się na wskaźnik do jej początku, ale zawiera niemutowalne dane. Lepiej
void g(const int* x); //efektywnie to samo co wyżej 

W obu przypadkach powyżej możesz przekazać tablicę o dowolnym rozmiarze. Ty wiesz, że masz 3x3, ale dla ogólnego przypadku - nie masz tego jak sprawdzić.
Natomiast:
void f(const int (&x)[3]); Zauważ &x: to to jest referencja do tablicy. Nadal nie przekazujesz kopii, ale
a. masz bounds check za darmo. Przekaż tablicę o rozmiarze 2, dostaniesz błąd kompilacji.
b. jej (nie)mutowalność załatwia const

Demo
Zakomentowane są compile errory, odkomentuj to zobaczysz o co mi chodzi.

0

Oto wersja OOP, czyli ten sam efekt, ale 2.5 razy więcej kodu ;)

#include <iostream>
#include <stdlib.h>
#include <stdio.h>

using namespace std;

class Game
{
private:
    int x;
    int y;

public:
    char player;
    int round;
    bool win;

    void playerMove(char player, char board[3][3])
    {
        char selection;
        int index;

        cout << "Player " << player << ", ";
        do
        {
            cout << "enter your index [1-9]: ";
            cin >> selection;
            index = selection - 48;
            decodeIndex(index);
        }
        while ((selection < 49) || (selection > 57) || board[x][y] != ' ');
        board[x][y] = (player == 'X') ? 'X' : 'O';
    }

    void checkResult(bool & win, int round, char board[3][3], char player)
    {
        if (checkMatch(board))
        {
            cout << "End of game, player " << player << " has won!" << endl;
            win = true;
        }
        else if (round == 9)
        {
            cout << "End of game, no winner!" << endl;
            win = true;
        }

    }

    bool checkMatch(char board[3][3])
    {
        if
        (
       ((board[0][0] == board[0][1]) && (board[0][1] == board[0][2]) && (board[0][0] != ' ')) ||
       ((board[1][0] == board[1][1]) && (board[1][1] == board[1][2]) && (board[1][0] != ' ')) ||
       ((board[2][0] == board[2][1]) && (board[2][1] == board[2][2]) && (board[2][0] != ' ')) ||
       ((board[0][0] == board[1][0]) && (board[1][0] == board[2][0]) && (board[0][0] != ' ')) ||
       ((board[0][1] == board[1][1]) && (board[1][1] == board[1][2]) && (board[0][1] != ' ')) ||
       ((board[0][2] == board[1][2]) && (board[1][2] == board[2][2]) && (board[0][2] != ' ')) ||
       ((board[0][0] == board[1][1]) && (board[1][1] == board[2][2]) && (board[0][0] != ' ')) ||
       ((board[0][2] == board[1][1]) && (board[1][1] == board[2][0]) && (board[0][2] != ' '))
        )
       {
            return true;
       }
       else
       {
           return false;
       }
    }

    void decodeIndex(int index)
    {
        switch (index)
        {
            case 1:
            {
                x = 0;
                y = 0;
            }
            break;

            case 2:
            {
                x = 0;
                y = 1;
            }
            break;

            case 3:
            {
                x = 0;
                y = 2;
            }
            break;

            case 4:
            {
                x = 1;
                y = 0;
            }
            break;

            case 5:
            {
                x = 1;
                y = 1;
            }
            break;

            case 6:
            {
                x = 1;
                y = 2;
            }
            break;

            case 7:
            {
                x = 2;
                y = 0;
            }
            break;

            case 8:
            {
                x = 2;
                y = 1;
            }
            break;

            case 9:
            {
                x = 2;
                y = 2;
            }
            break;
        }
    }
};

class Board
{
public:
    char board[3][3];

    void display(char board[3][3])
    {
        cout << "-------------------" << endl;
        cout << "|  " << board[0][0] << "  |  " <<board[0][1] << "  |  " << board[0][2] << "  |" << endl;
        cout << "-------------------" << endl;
        cout << "|  " << board[1][0] << "  |  " << board[1][1] << "  |  " << board[1][2] << "  |" << endl;
        cout << "-------------------" << endl;
        cout << "|  " << board[2][0] << "  |  " << board[2][1] << "  |  " << board[2][2] << "  |" << endl;
        cout << "-------------------" << endl << endl;
    }

    void prepare(char ch, char board[3][3])
    {
        board[0][0] = ch;
        board[0][1] = ch;
        board[0][2] = ch;
        board[1][0] = ch;
        board[1][1] = ch;
        board[1][2] = ch;
        board[2][0] = ch;
        board[2][1] = ch;
        board[2][2] = ch;
    }
};

int main()
{
    Game newGame;
    newGame.round = 0;
    newGame.win = false;

    Board newBoard;
    newBoard.prepare(' ', newBoard.board);

    while(!newGame.win)
    {
        newBoard.display(newBoard.board);
        newGame.checkResult(newGame.win, newGame.round, newBoard.board, newGame.player);
        if (!newGame.win)
        {
            newGame.player = (newGame.round % 2 == 0) ? 'X' : 'Y';
            newGame.playerMove(newGame.player, newBoard.board);
            newGame.round++;
        }
        getchar();
        system("cls");
    }
    return 0;
}

0
newGame.round = 0;
newGame.win = false;

Czemu to ustawiasz ręcznie w ten sposób, zamiast skorzystać z konstruktora? Rzuć okiem na:
https://4programmers.net/Forum/C_i_C++/257872-konstruktor_
https://cpp0x.pl/kursy/Kurs-C++/Dodatkowe-materialy/Konstruktor-destruktor-i-konstruktor-kopiujacy-klasy/313

1

Ja wiem, że to mikroprojekt, ale dwie rzeczy mi się nie podobają, z czego jedna jest dość uniwersalna:

  1. Pomieszanie IO z logiką.
  2. Trwanie przy starym C++. Sam kastrujesz ten kod z ułatwień. Jeżeli celowo chcesz pisać w C++03 to ok, zamykam gębę, ale jeśli nie - spróbuj w 17 albo chociaż 11 ;p.

Z rzeczy bardziej szczegółowych - mieszasz pola publiczne z prywatnymi. To niby nie jest błąd w ścisłym tego słowa rozumieniu...ale jak dla mnie kiepsko wyraża intencje. No i nadal gołą tablicę przekazujesz. Czy jesteś pewien że o to chodzi?

0

Pomieszanie IO z logiką.

Co to znaczy?

Trwanie przy starym C++. Sam kastrujesz ten kod z ułatwień. Jeżeli celowo chcesz pisać w C++03 to ok, zamykam gębę, ale jeśli nie - spróbuj w 17 albo chociaż 11 ;p.

Kompilatora do C++ (Code Blocks) nie uruchamiałem przez rok. Co więcej, zaskoczony jestem tym, że w tym języku jest operator lambda. A ten znam z JS. Gdzie najlepiej sprawdzić aktualną specyfikację C++? Chętnie poczytam o nowościach.

mieszasz pola publiczne z prywatnymi.

Zapewne masz na myśli zmienne x i y. Tak bardziej w ramach eksperymentu dałem je jako private.

No i nadal gołą tablicę przekazujesz. Czy jesteś pewien że o to chodzi?

Póki co jestem na takim poziomie, że zapisów typu:

void f(const int (&x)[3]); 
std::optional<Board> boardAfterMove(const Board& board, const Player& player, std::size_t field).

Nie czuję. Pierwsze moje skojarzenie widząc nawiasy ostre? Typ generyczny. Ale to nie to.

0

Specyfikacja jest tu:
http://www.cppreference.com od standardu C++98 do C++23 (jeszcze w powijakach ;) )
Z książek polecam Effective Modern C++ Steve'a Meyersa, on nie wychodzi ponad 14 ale to nadal sporo czytania ;P

std::optional<Board> boardAfterMove(const Board& board, const Player& player, std::size_t field)
Noooo, tak optional to szablon klasy, który tu specjalizujesz dla planszy. I on jest albo "odpakowywalny" (czyli zawiera prawidłowy board) albo pusty.

EDIT: pomieszane IO z logiką... no to pomieszane IO z logiką. ;)
Np. to:

    void playerMove(char player, char board[3][3])
    {
        char selection;
        int index;

        cout << "Player " << player << ", "; //tu
        do
        {
            cout << "enter your index [1-9]: "; //I tu
            cin >> selection; //i tu 
            index = selection - 48;
            decodeIndex(index);
        }
        while ((selection < 49) || (selection > 57) || board[x][y] != ' ');
        board[x][y] = (player == 'X') ? 'X' : 'O';
    }

Imho to powinno być
a. bardziej podzielone, wtedy:
b. możesz dla mniejszych klas potworzyć operatory << i >> co z kolei spowoduje:
c. że będziesz mógł w strumień opakować np. socket i grać po sieci, ergo logika gry będzie bardziej reużywalna i niezależna od operacji we/wy. Albo po prostu testować logikę osobno.
Tzn. ja wiem, że mi się łatwo gada, a do tego jest mikra ilość kodu, która przez potrzebny boilerplate Ci monstrualnie urośnie ;P
Ale spróbuj

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