[Avis & Amélioration] Morpion

**thibzko** · 31/12/2014, 14h08

Salut à tous, étant débutant dans ce langage, je souhaiterai avoir des avis sur mon code(améliorations du code, organisation etc.).

Je vous laisse mon code source afin de voir ce qu'il en est:

Main.cpp

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
 
#include <iostream>
#include "Game.h"
 
using namespace std;
 
int main(){
 
	Game g;
	g.play();
 
	return 0;
}

Game.h

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
 
#ifndef GAME_H
#define GAME_H
 
#include <array>
 
enum PlayerChar {
    Empty, 	// Empty cell
    Round, 	// Round piece / player1
    Cross 	// Cross piece / player2
};
 
class Game {
	public:
		Game();
		void play();						// Lance le jeu(appel des différentes méthodes)
 
	private:
		void resetBoard(); 					// Efface le plateau
		void printBoard() const; 			// Affiche le plateau
		void switchPlayer(); 				// Change de joueur après avoir jouer
		char getPlayer(PlayerChar& p); 	// Retourne le joueur courant
		void putPiece();					// Place piece du joueur
		bool isSlotEmpty(int slot) const;	// Retourne TRUE si le slot est vide
		bool isBoardFull() const;			// Retourne TRUE si le plateau est plein
		bool checkLine() const;			 	// Retourne TRUE si ligne gagnante
		bool checkColumn() const;			// Retourne TRUE si colonne gagnante
		bool checkDiagonal() const;		// Retourne TRUE si diagonale gagnante
		bool checkWon() const;				// Retourne TRUE si le jeu est gagné
 
		std::array<PlayerChar, 9> mBoard; 	// Plateau de jeu
		PlayerChar mPlayer; 				// Joueur courrant
		int mTurn; 							// Nombre de tours joués
};
 
#endif // GAME_H

Game.cpp

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
 
#include "Game.h"
#include <iostream>
 
Game::Game(): mPlayer(Round), mTurn(1){
	resetBoard();
}
 
void Game::resetBoard() {
	mBoard.fill(Empty);
}
 
void Game::printBoard() const {
	std::cout << std::endl << std::endl;
	if(checkWon())
		std::cout << "  VICTOIRE" << std::endl;
	else
		std::cout << "   Tour  n°" << mTurn << std::endl;
	std::cout << "+---+---+---+" << std::endl;
	for(size_t i = 0; i < mBoard.max_size(); i++) {
		std::cout << "| ";
		switch(mBoard[i]) {
			case Round:
				std::cout << " O ";
				break;
			case Cross:
				std::cout << " X ";
				break;
			case Empty:
				std::cout << " " << i+1 << " ";
				break;
			default:
				break;
 
		}
		std::cout << " ";
		if ((i+1) % 3 == 0 && i != 0) {
			std::cout << "|" << std::endl;
			std::cout << "+---+---+---+" << std::endl;
		}
	}
}
 
void Game::play() {
 
	while(!isBoardFull() && !checkWon()) {
		printBoard();
		putPiece();
		switchPlayer();
		mTurn++;
	}
	switchPlayer();
	printBoard();
	std::cout << std::endl;
	if(isBoardFull() && !checkWon())
		std::cout << "Match nul !";
	if(checkWon()) {
		std::cout << "Joueur [";
		std::cout << getPlayer(mPlayer);
		std::cout << "]";
		std::cout << " a gagné la partie en ";
		std::cout << mTurn - 1 << " coups !";
	}
	std::cout << std::endl;
}
 
void Game::switchPlayer() {
	if(mPlayer == Cross) mPlayer = Round;
	else mPlayer = Cross;
}
 
char Game::getPlayer(PlayerChar& p) {
	return( (p == Cross) ? 'X' : 'O');
}
 
void Game::putPiece() {
	std::cout << "Joueur [";
	std::cout << getPlayer(mPlayer);
	std::cout << "]" << std::endl;
	int slot(-1);
	while(slot < 1 || slot > 9 || !isSlotEmpty(slot - 1)) {
		std::cout << "Case: ";
		std::cin >> slot;
	}
	mBoard[slot - 1] = mPlayer;
}
 
bool Game::isSlotEmpty(int slot) const {
	return(mBoard[slot] == Empty);
}
 
bool Game::checkLine() const {
	bool res = false;
	for(size_t i = 0; i < mBoard.max_size(); i += 3) {
		if(!isSlotEmpty(i)) {
			if(mBoard[i] == mBoard[i+1] && mBoard[i] == mBoard[i+2]) {
				res = true;
			}
		}
	}
	return res;
}
 
bool Game::checkColumn() const {
	bool res = false;
	for(size_t i = 0; i < 3; i++) {
		if(!isSlotEmpty(i)) {
			if(mBoard[i] == mBoard[i+3] && mBoard[i] == mBoard[i+6]) {
				res = true;
			}
		}
	}
	return res;
}
 
bool Game::checkDiagonal() const {
	bool res = false;
	for(size_t i = 0; i < mBoard.max_size(); i++) {
		if(!isSlotEmpty(0)) {
			if(mBoard[0] == mBoard[4] && mBoard[0] == mBoard[8]) {
				res = true;
			}
		}
		if(!isSlotEmpty(2)) {
			if(mBoard[2] == mBoard[4] && mBoard[2] == mBoard[6]) {
				res = true;
			}
		}
	}
	return res;
}
 
bool Game::isBoardFull() const {
	bool res = true;
	for(size_t i = 0; i < mBoard.max_size(); i++) {
		if(mBoard[i] == Empty) {
			res = false;
		}
	}
	return res;
}
 
bool Game::checkWon() const {
	if(checkLine() || checkDiagonal() || checkColumn()) {
		return true;
	} else {
		return false;
	}
}

Merci et bonne journée,

**oodini** · 31/12/2014, 14h32

Je n'ai pas été dans le détail du code, mais un rapide coup d’œil est satisfaisant : encapsulation, utilisation de la constance, effort d'utilisation du paradigme C++ (array plutôt que [])...

Rien de choquant, mais tu peux chercher à faire un peu mieux :

dans Game::checkWon(), tu peux renvoyer directement le résultat de ton opération booléenne
dans Game::isBoardFull(), tu peux utiliser std::find_if (ou std::find_if_not)
dans tes fonctions check, il y a peut-être moyen de factoriser, mais je n'ai pas le temps de me pencher sur la logique

Sinon, commentaire plus subjectif : en C++, les accolades ouvrantes se trouvent généralement en début de ligne (style Allman/BSD), bien que le créateur du C++, Stroustrup, utilise comme toi le style K&R (remanié).

**thibzko** · 31/12/2014, 15h36

Merci pour ta réponse. J'ai en effet modifié Game::checkWon() afin de retourner directement le résultat.
Par contre, j'ai regarder pour implémenter std::find_if mais je ne comprends pas bien comment cela fonctionne.

Dans certains exemples, je vois que ça peut retourner un booléen, dans d'autre, ça retourne l'élément trouvé ?
Je ne suis pas sur de ce que j'ai compris, si quelqu'un peut m'expliquer plus en détail ?

Sinon pour ce qui est des accolades, j'ai pris l'habitude de les mettre de cette façon, je trouve ça mieux. Enfin bon, c'est une question de goûts

**oodini** · 31/12/2014, 16h55

Envoyé par thibzko

Par contre, j'ai regarder pour implémenter std::find_if mais je ne comprends pas bien comment cela fonctionne.
Dans certains exemples, je vois que ça peut retourner un booléen, dans d'autre, ça retourne l'élément trouvé ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

return (std::find_if(mBoard.cbegin(), mBoard.cend(), std::bind(equal_to<int>(), _1, Empty)) != mBoard.cend());

D'une grande élégance, non ?

Pas testé, mais ça doit être quelque chose de ce goût là.

**thibzko** · 31/12/2014, 17h01

Envoyé par oodini

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

return (std::find_if(mBoard.cbegin(), mBoard.cend(), std::bind(equal_to<int>(), _1, Empty)) != mBoard.cend());

D'une grande élégance, non ?

Pas testé, mais ça doit être quelque chose de ce goût là.

Le compilo me retourne une erreur, il ne connait pas std::bind, n'y le _1.
Outre ce fait, est-ce vraiment mieux d'utiliser quelque chose comme ça, plotôt qu'un simple parcours de l'array ?

**oodini** · 31/12/2014, 17h27

Envoyé par thibzko

Le compilo me retourne une erreur, il ne connait pas std::bind, n'y le _1.

Comme tu utilisais array, j'ai supposé que tu utilisais C++11.
Si ce n'est pas le cas, utilise bind1st ou bind2d :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

std::bind1st(isEqual, Empty)

Envoyé par thibzko

Outre ce fait, est-ce vraiment mieux d'utiliser quelque chose comme ça, plutôt qu'un simple parcours de l'array ?

Plus dur à lire, mais moins verbeux.
Et surtout, ça t'apprend à avoir le réflexe d'aller voir ans dans la STL.

**JolyLoic** · 01/01/2015, 15h58

Envoyé par oodini

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

return (std::find_if(mBoard.cbegin(), mBoard.cend(), std::bind(equal_to<int>(), _1, Empty)) != mBoard.cend());

D'une grande élégance, non ?

La même avec les lambdas (je trouve ça plus clair):

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

return (std::find_if(mBoard.cbegin(), mBoard.cend(), [](PlayerChar pc){return pc == Empty;}) != mBoard.cend());

Ou, tant qu'à utiliser un algorithme, autant en utiliser un qui explicite ce qu'on veut :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

return all_of(mBoard.cbegin(), mBoard.cend(), [](PlayerChar pc){return pc != Empty;});

[Avis & Amélioration] Morpion

C++

Vue hybride

Discussions similaires

Partager

Partager