[C++] Gioco Di Carte

Super Squirrel
Prima di cimentarmi in cose più difficili ho provato ad implementare il gioco della scopa. Ecco il codice:



Il programma sembra funzionare, ma vorrei sapere se il modo in cui l'ho impostato va bene o potrebbe essere migliorato? Voi come avreste fatto?
Anche se quello che mi interessa maggiormente è l'impostazione generale, qualsiasi consiglio su come ottimizzare le singole funzioni è ben accetto.

Risposte
claudio862
Allora, ovviamente non mi metterò a testare e correggere interamente il tuo programma (non gratis, almeno :D). L'ho provato e crasha alla fine della prima mano per un errore di array out of boundary. Comunque qualche commento:


    [*:1hd5cra9]
    template <typename T>
    void scambia(T& a, T& b)
    {
        T temp = a;
        a = b;
        b = temp;
    }

    Questa funzione esiste già: std::swap (e sarebbe anche meglio perché sfrutta la move semantic, ma non ha granché rilevanza qui).
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    srand(time(NULL));

    Questo va chiamato solo una volta. Mettilo all'inizio del main, non dentro la funzione "mischia". Inoltre potrebbe essere meglio usare srand(0), così il generatore di numeri casuali dà risultati consistenti ed è più semplice replicare i problemi (ovviamente solo quando sviluppi, non nel programma finale).
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Sarebbe meglio usare un enum per il seme delle carte, invece che un numero.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Non usare variabili globali. Passale come argumento alle funzioni, oppure come variabili membri delle classi.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Non dichiarare più di una variabile per linea.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Alcune funzioni sono troppo lunghe. Spezzale. Ad esempio, quando calcoli i punti, puoi creare una funzione per ogni "categoria" di punti (scope, primiera...)
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    _turno->ultima_presa = 1;
    ...
    if(g1.ultima_presa == 1)

    "ultima_presa" è un bool, perché lo confronti con 1? Usa

    _turno->ultima_presa = true;
    ...
    if(g1.ultima_presa)

    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Perché passi i giocatori come puntatori? Se non usi aritmetica dei puntatori non hai bisogno di puntatori. Passali come riferimenti.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Usi l'underscore all'inizio di alcune variabili, ma non in modo consistente. In passato si usava per distinguere variabili membro da quelle locali. Gli editor / IDE moderni si solito usano colori diversi, quindi non è più necessario.
    Inoltre rischi di usare un nome riservato. In C e C++ nomi che iniziano con underscore e una lettera maiuscola (come purtroppo fanno metà delle guide nelle include guards) o che contengono due underscore consecutivi sono riservati. In genere non è un problema reale, ma questo non è un buon motivo per usarli.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Usi un sacco di unsigned int. In alcuni casi fai bene, soprattutto nei cicli for dove li compari con vector.size(), che è unsigned. Purtroppo rischi di esporti ad alcuni bug (ad esempio se sottrai due variabili unsigned e il risultato è negativo).
    In generale, il tipo "predefinito" è int, dovresti usare quello a meno di precise motivazioni (ad esempio bit shifting e overflow, che sono definiti solo per tipi unsigned).
    D'altro canto ci sono diversi punti di vista su questo argomento. C'è chi sostiene che se una variabile non può essere negativa si dovrebbe usare un tipo unsigned. Non c'è una risposta esatta.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Il tipo "struct carta" è semplicemente un contenitore. Non ha alcuna logica al suo interno, semplicemente raggruppa un seme e un numero.
    Il tipo "class giocatore" invece no. Ad esempio potrebbe avere una funzione membro che calcola i punti. Ma quindi non dovresti accedere alle sue variabili da fuori.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    Infine, indentazione e spaziatura sono corrette e consistenti. Complimenti! È molto più di qualsiasi principiante (ma anche molti esperti...) che abbia mai visto.
    [/*:m:1hd5cra9]
    [*:1hd5cra9]
    A latere, ma questo è solo un mio personale consiglio, dovresti scrivere codice in inglese. Linguaggio e librerie sono in inglese, stona davvero molto avere variabili e funzioni in italiano. Inoltre ti apri alla comunità internazionale (a cui puoi chiedere consigli), Anche se in questo specifico caso la scopa potrebbe essere un gioco tradizionalmente italiano, magari non ci sono traduzioni per "premiera" o "settebello" :D[/*:m:1hd5cra9][/list:u:1hd5cra9]

Super Squirrel
Innanzitutto grazie per aver risposto!

L'ho provato e crasha alla fine della prima mano per un errore di array out of boundary.


Strano, a me funziona perfettamente. Oltre a fare qualche partita di persona, l'ho testato impostando la variabile indice a 0 e ha concluso ogni partita senza dare errori.

Usi un sacco di unsigned int. In alcuni casi fai bene, soprattutto nei cicli for dove li compari con vector.size(), che è unsigned. Purtroppo rischi di esporti ad alcuni bug (ad esempio se sottrai due variabili unsigned e il risultato è negativo).


Certo, bisogna prestare un po' di attenzione in più, ma credo sia meglio usare unsigned se la variabile non può assumere valori negativi. Uno dei bug che ho dovuto risolvere è il seguente... detta n la dimensione di un array v, se utilizzo int il codice è:

for(int i = n - 1; i >= 0; i--)
{
    cout << v[i] << "\t";
}


se invece voglio utilizzare unsigned int ho risolto nel seguente modo:

for(unsigned int i = n; i > 0; i--)
{
    cout << v[i - 1] << "\t";
}


Non usare variabili globali. Passale come argumento alle funzioni, oppure come variabili membri delle classi.


Dove dovrei dichiarare allora le seguenti variabili?

vector <carta> mazzo, terreno;
const unsigned int Bastoni = 10, Spade = 11, Coppe = 12, Denari = 14;


Nel main e poi passarle ogni volta alle funzioni?

Il tipo "struct carta" è semplicemente un contenitore. Non ha alcuna logica al suo interno, semplicemente raggruppa un seme e un numero.
Il tipo "class giocatore" invece no. Ad esempio potrebbe avere una funzione membro che calcola i punti. Ma quindi non dovresti accedere alle sue variabili da fuori.


In effetti la classe giocatore può essere sfruttata meglio. Ci rifletterò.

Perché passi i giocatori come puntatori? Se non usi aritmetica dei puntatori non hai bisogno di puntatori. Passali come riferimenti.


Lo facevo per alternare il giocatore che tira per primo. Ora passo tutto per riferimento dopo aver modificato il main nel seguente modo:

int main()
{
    srand(time(0));
    unsigned int partita = 0, vittoria = 11;
    cout << "INSERIRE IL NOME DEL PRIMO GIOCATORE: ";
    cin >> g1.identificatore;
    cout << "INSERIRE IL NOME DEL SECONDO GIOCATORE: ";
    cin >> g2.identificatore;
    while((g1.punti < vittoria && g2.punti < vittoria) || g1.punti == g2.punti)
    {
        ++partita;
        cout << endl << "PARTITA " << partita << endl;
        if(partita % 2 == 1)
        {
            game(g1, g2);
        }
        else
        {
            game(g2, g1);
        }
        cout << endl << "PUNTEGGIO:" << endl;
        cout << g1.identificatore << " = " << g1.punti << endl;
        cout << g2.identificatore << " = " << g2.punti << endl;
    }
    if(g1.punti > g2.punti)
    {
        cout << endl << g1.identificatore;
    }
    else
    {
        cout << endl << g2.identificatore;
    }
    cout << " VINCE!";


Per quanto riguarda gli altri consigli ne terrò conto! :D

apatriarca
Alcuni commenti:

1. Come già detto da @claudio86, non c'è alcuna ragione di ridefinire funzioni già presenti nello standard come std::swap.

2. La funzione mischia non è corretta in quanto la probabilità associata ad ogni permutazione non è uniforme. Un algoritmo semplice e ottimale per questo tipo di problemi è Fisher-Yates. Per una descrizione dei problemi puoi ad esempio guardare questo articolo.

3. Non mi è chiara la ragione per cui i valori che hai associato ad ogni seme partono da 10. Ci sarebbero in effetti alcuni vantaggi nell'algoritmo ad usare 0, 1, 2 e 3 (o eventualmente 1, 2, 3, 4). Potresti anche usare un singolo numero intero nella forma seme*K + numero dove K è sufficientemente grande. Le potenze di due potrebbero essere particolarmente comode perché ti permetterebbero di usare operazioni bit-wise per estrarre i valori invece di divisioni/resti.

4. Non mi è chiaro perché mischi due volte. Una volta è sufficiente.

Non ho ancora dato una occhiata al codice per la partita di scopa. Dopo provo a darci una occhiata migliore.

vict85
Seppur condivida in linea di massima i commenti precedenti trovo che falliscano nell'esprimere il problema di fondo: la struttura del codice risponde in modo inadeguato al problema che ti stai ponendo. Inoltre, la mancanza di una buona suddivisione in file, il mescolare in modo maldestro C e C++, e il mancato uso delle più comuni librerie standard non farebbero una buona impressione in un colloquio aziendale. Inoltre è un errore grave usare direttamente un qualsiasi input dall'utente senza controllarne la validità (il bug a cui si riferiva Claudio è a riga 341 dove usi una variabile insicura).

Un problema della tua struttura è la gestione dei dati in modo apparentemente casuale. Per esempio i tre dati di gioco (mazzo, tavolo e giocatori) li definisci senza alcun senso visibile. Inoltre considerando che il mazzo possiede sempre 40 carte (per lo meno nel modo in cui gestirei la cosa io) e che il tavolo può avere al massimo 13 carte al suo interno, userei direttamente dei vettori di dimensione fissa. Per esempio io avrei gestito la cosa usando una struttura tipo
class carta;     // da definire
class giocatore; // da definire
class gioco
{
  public:
    // costruttori e varie funzioni che gestiscono il gioco
  private:
    std::unique_ptr<giocatore> giocatori[2]; // o anche puntatori normali o direttamente oggetti di tipo giocatore
    carta deck[40];                          // o usando std::array
    carta table[13];                         // o usando std::array
    uint8_t next_card_idx;
};
e facendo gestire tutto il gioco alla classe (il puntatore this ti risolve il passaggio del mazzo e del tavolo alle varie funzioni).
Un altro aspetto discutibile è rappresentare le carte con 2 unsigned int anche se ci sono solo 40 carte (6 bit sono sufficienti per memorizzare 40 valori diversi). Pertanto trovo che usare più di due uint8_t sia uno spreco di risorse e non è difficile usarne soltanto uno (anche se bisogna fare operazioni aritmetiche o sui bit per ricavare il seme).
Simile discorso per il giocatore: ha bisogno solo di 3 carte per la mano e vari uint8_t che contengono il punteggio totale attuale e qualche informazioni parziale (numero totale di carte nel mazzetto, numero di denari nel mazzerro, carta di maggiore valore per ognuno dei 4 semi e via dicendo[nota]Il sette bello non è necessario perché puoi usare i valori parziali della primiera per determinarlo[/nota]). Insomma probabilmente sono sufficienti una decina di byte più la stringa, mentre tu stai usando fino a 40 volte la dimensione di una carta.
In pratica usi molta più memoria di quanto è necessarie e non fai troppa attenzione allo scope delle varie variabili che usi.

Una questione minore è nell'utilizzo di windows.h per rappresentare gli elementi con i colori. Per quanto possa essere simpatico penso che per l'utente sia meglio vedere le carte come 3♦, A♣, J♥, 2♠, K♥ piuttosto che le versioni colorate in cui è difficile capire quali siano i vari semi. Similmente trovo che sarebbe più intuitivo dire qualche carta vuoi piuttosto che il suo indice (ovviamente in questo caso non devi usare una qualche lettera per rappresentare i semi e non un simbolo difficile da inserire come ♦ o ♣).

Super Squirrel
1. Come già detto da @claudio86, non c'è alcuna ragione di ridefinire funzioni già presenti nello standard come std::swap.


Semplicemente non ero a conoscenza della sua esistenza.

2. La funzione mischia non è corretta in quanto la probabilità associata ad ogni permutazione non è uniforme. Un algoritmo semplice e ottimale per questo tipo di problemi è Fisher-Yates. Per una descrizione dei problemi puoi ad esempio guardare questo articolo.

4. Non mi è chiaro perché mischi due volte. Una volta è sufficiente.


In effetti per la funzione mischia ho utilizzato la prima idea che mi è venuta in mente, ma avendo intuito, per il modo in cui avvengono gli scambi, che alcune sequenze fossero più probabili di altre, ho pensato di compensare richiamando la funzione mischia due volte.
Con l'algoritmo di Fisher-Yates, invece, pur essendo altrettanto semplice, ogni sequenza è perfettamente casuale.

3. Non mi è chiara la ragione per cui i valori che hai associato ad ogni seme partono da 10.


Semplicemente sono i numeri associati al colore dell'output che ho scelto per i vari semi.

Seppur condivida in linea di massima i commenti precedenti trovo che falliscano nell'esprimere il problema di fondo: la struttura del codice risponde in modo inadeguato al problema che ti stai ponendo. Inoltre, la mancanza di una buona suddivisione in file, il mescolare in modo maldestro C e C++, e il mancato uso delle più comuni librerie standard non farebbero una buona impressione in un colloquio aziendale. Inoltre è un errore grave usare direttamente un qualsiasi input dall'utente senza controllarne la validità (il bug a cui si riferiva Claudio è a riga 341 dove usi una variabile insicura).


La suddivisione in file e il controllo dell'input li ho ritenuti secondari al momento e quindi li ho tralasciati.
Cosa intendi con mescolare C e C++?
Come si è visto nel caso della funzione swap, non ho una grande conoscenza della std, in ogni caso a quali librerie standard ti riferisci?

Un problema della tua struttura è la gestione dei dati in modo apparentemente casuale. Per esempio i tre dati di gioco (mazzo, tavolo e giocatori) li definisci senza alcun senso visibile. Inoltre considerando che il mazzo possiede sempre 40 carte (per lo meno nel modo in cui gestirei la cosa io) e che il tavolo può avere al massimo 13 carte al suo interno, userei direttamente dei vettori di dimensione fissa. Per esempio io avrei gestito la cosa usando una struttura tipo
CODICE:
class carta; // da definire
class giocatore; // da definire
class gioco
{
public:
// costruttori e varie funzioni che gestiscono il gioco
private:
std::unique_ptr giocatori[2]; // o anche puntatori normali o direttamente oggetti di tipo giocatore
carta deck[40]; // o usando std::array
carta table[13]; // o usando std::array
uint8_t next_card_idx;
};
e facendo gestire tutto il gioco alla classe (il puntatore this ti risolve il passaggio del mazzo e del tavolo alle varie funzioni).


Mi rendo conto che il tutto poteva essere strutturato parecchio meglio, proverò a sviluppare l'idea di una classe gioco.
Per il mazzo non ho usato un semplice array visto che mi serve conoscere il "size" attuale, e quindi ho ritenuto più semplice utilizzare la classe vector che dichiarare variabili che tenessero conto della "dimensione attuale".

Un altro aspetto discutibile è rappresentare le carte con 2 unsigned int anche se ci sono solo 40 carte (6 bit sono sufficienti per memorizzare 40 valori diversi). Pertanto trovo che usare più di due uint8_t sia uno spreco di risorse e non è difficile usarne soltanto uno (anche se bisogna fare operazioni aritmetiche o sui bit per ricavare il seme).
Simile discorso per il giocatore: ha bisogno solo di 3 carte per la mano e vari uint8_t che contengono il punteggio totale attuale e qualche informazioni parziale (numero totale di carte nel mazzetto, numero di denari nel mazzerro, carta di maggiore valore per ognuno dei 4 semi e via dicendo1). Insomma probabilmente sono sufficienti una decina di byte più la stringa, mentre tu stai usando fino a 40 volte la dimensione di una carta.
In pratica usi molta più memoria di quanto è necessarie e non fai troppa attenzione allo scope delle varie variabili che usi


Non ho proprio pensato ad ottimizzare il programma in questi termini ed inoltre non ero a conoscenza di questi int "più leggeri".

Non ho ancora dato una occhiata al codice per la partita di scopa. Dopo provo a darci una occhiata migliore.


Devo ancora aggiustare parecchie cose, ecco comunque il codice con qualche piccola correzione:

EDIT: apportati altri piccoli aggiustamenti e corretta la funzione mischia


apatriarca
Se quei codici hanno un significato allora è importante che questo significato sia visibile nel codice. Se tra qualche mese riprenderai questo codice non ricorderai di certo il significato di 10.. Siccome stai includendo windows.h in ogni caso tanto vale utilizzare le costanti relative. Per esempio, rosso su sfondo bianco può essere definito come segue:

BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED | FOREGROUND_RED

Sinceramente utilizzerei un array costante per mappare i semi ai colori e usare valori da 0 a 3 per i semi. Il vantaggio è che il corpo del ciclo in prepara_mazzo può essere semplificato scrivendo mazzo[ i ].seme = i / 10 e mazzo.numero = i % 10. Codici simili possono essere usati nel resto del codice.

In realtà anche la funzione mischia è già presente nella libreria standard (la funzione si chiama random_shuffle o shuffle e si trova in algorithm).

Super Squirrel
La parte sulle costanti relative non l'ho capita, per quanto riguarda gli altri due consigli ne terrò conto.

Tornando alla struttura generale del programma, convince poco anche me. Ho provato a strutturarlo diversamente, ecco il codice:



Va meglio rispetto all'altra versione?
Ovviamente non pretendo il codice, ma potreste dirmi in modo un po' più dettagliato voi come avreste strutturato il tutto? tipo quale classe o funzione dovrebbe occuparsi delle varie fasi del gioco.

apatriarca
Ogni bit del secondo argomento di SetConsoleTextAttribute ha un particolare significato in termini di colore e per semplificarne la definizione esistono delle macro che vanno combinate usando l'operatore or |. Puoi inserire anche direttamente i valori, ma è in generale più semplice e leggibile fare uso di queste macro. Una tipica chiamata alla funzione prende la forma
SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_GREEN | BACKGROUND_RED);

In questo caso stai settando un testo verde sopra uno sfondo rosso.

apatriarca
Una delle lezioni più importanti che ho imparato programmando professionalmente è che all'inizio di un progetto si hanno raramente tutte le informazioni necessarie per fare le scelte corrette in fase di implementazione. In effetti, i casi in cui conosciamo già tutto sono quelli in cui abbiamo già implementato qualcosa di simile. Per questa ragione sono un po' restio a fornirti opinioni sulla struttura che un codice del genere dovrebbe avere. Ti invito invece a cercare di arricchire il codice con funzionalità ulteriori in modo da renderti conto da solo degli eventuali problemi e limiti della struttura da te scelta.

Alcune idee potrebbero essere le seguenti (ho inserito la difficoltà di ognuna):
1. [SEMPLICE] Supporto di un numero variabile di giocatori. La scopa si può infatti giocare in due, tre, quattro (a coppie) e (almeno secondo Wikipedia) sei (a squadre di tre).
2. [DIFFICILE] Gran parte del gioco della scopa consiste nel ricordarsi le carte uscite in modo da prevedere quali carte potrebbe avere il tuo avversario. Ma in un programma come questo ogni giocatore può guardare le carte dell'altro. Una possibile evoluzione potrebbe quindi essere quella di implementare partite in una rete locale.
3. [MEDIA] Creare una intelligenza artificiale per uno dei due giocatori.
4. [DIFFICILE] Creare una interfaccia grafica per il programma.
5. [FACILE] Permettere ai giocatori di giocare a delle varianti del gioco (aggiungendo o togliendo regole o modifiche).

Super Squirrel
Capisco, proverò a seguire il tuo suggerimento.

I punti 1, 3 e 5 sono molto stimolanti e risulta evidente che richiedano delle modifiche alla struttura del programma.

Per quanto riguarda il punto 2 per simulare una vera partita avevo pensato di usare un certo numero di endl visto che di reti non so praticamente nulla. Nel caso che argomenti bisognerebbe studiare per realizzare una cosa del genere.

Per quanto riguarda l'interfaccia grafica, visto che non so nulla al riguardo, avevo già intenzione di fare alcune domande. Si può sviluppare un'interfaccia grafica solo col C++ ? Nel caso, vale la pena studiare per farlo partendo da zero o meglio imparare direttamente ad usare qualche strumento/libreria già esistente?

vict85
Che cosa intendi con "solo col C++"? Usare librerie C++ come wxwidget o Qt rientra nell'usare solo il C++? Usando solamente le librerie standard è impossibile, ma immagino che tu possa farlo usando solo windows.h (ovviamente il software andrebbe solo su windows).

apatriarca
Il C++ non fornisce tra le sue librerie standard alcuna funzionalità per sviluppare interfacce grafiche. In effetti esistono parecchie funzionalità anche molto più base come ottenere l'elenco dei file in una cartella che non sono supportate nelle librerie standard del C++. E' possibile che in futuro si possano introdurre tali funzionalità, ma per il momento fare uso di librerie esterne è l'unica possibilità. Per la creazione di questa interfaccia esistono in effetti diverse alternative a seconda di come la vuoi realizzare, delle piattaforme su cui vuoi far girare la tua applicazione e delle tue preferenze. Si può andare dall'uso delle API di Windows, a qualcosa come Qt, alle OpenGL o DirectX.. Un passo intermedio potrebbe essere di realizzare una interfaccia testuale più avanzata usando le API di Windows o qualcosa come ncurses.

Per il punto 2 è necessario imparare a far comunicare diversi programmi tra di loro, probabilmente attraverso socket.. Imparare il funzionamento dei protocolli di rete può essere utile sia per la comprensione di possibili problemi, sia per progettare il particolare protocollo che userà il tuo programma durante la partita. Il C++ non è il linguaggio più semplice con cui realizzare questo tipo di applicazioni.. ma è certamente possibile.

Super Squirrel
Usare librerie C++ come wxwidget o Qt rientra nell'usare solo il C++?


Essendo per me un argomento completamente nuovo, ovviamente non conosco le suddette librerie, ma sarei curioso di sapere in quale linguaggio sono state implementate.

Il C++ non fornisce tra le sue librerie standard alcuna funzionalità per sviluppare interfacce grafiche. In effetti esistono parecchie funzionalità anche molto più base come ottenere l'elenco dei file in una cartella che non sono supportate nelle librerie standard del C++. E' possibile che in futuro si possano introdurre tali funzionalità, ma per il momento fare uso di librerie esterne è l'unica possibilità. Per la creazione di questa interfaccia esistono in effetti diverse alternative a seconda di come la vuoi realizzare, delle piattaforme su cui vuoi far girare la tua applicazione e delle tue preferenze. Si può andare dall'uso delle API di Windows, a qualcosa come Qt, alle OpenGL o DirectX.. Un passo intermedio potrebbe essere di realizzare una interfaccia testuale più avanzata usando le API di Windows o qualcosa come ncurses.


Capisco, a questo punto conviene fare qualche ricerca e iniziare da quello che mi convince maggiormente.

Rispondi
Per rispondere a questa discussione devi prima effettuare il login.