对代码有任何建议或提示吗?
#include <iostream>
#include <string>
#define GRID_SIZE 9
using namespace std;
void printGame(char gameGrid[]);
void inputToSlot(char gameGrid[], int slot, char choice);
void initialiseGame(char gameGrid[]);
bool isFull(char gameGrid[]);
bool isEmpty(char input);
bool checkWon(char gameGrid[]);
bool rowWon(char gameGrid[]);
bool columnWon(char gameGrid[]);
bool diagonalWon(char gameGrid[]);
int main(int argc, const char * argv[])
{
char gameGrid[GRID_SIZE];
initialiseGame(gameGrid);
int currentTurn = 0;
cout << "Tic Tac Toe by Bryan Lean" << endl << endl;
printGame(gameGrid);
cout << endl;
while (!isFull(gameGrid)) {
int slotChoice;
currentTurn += 1;
bool isP2 (currentTurn % 2 == 0);
// Prompt
if (isP2) {
cout << "Player 2 turn..." << endl << "Insert to slot > ";
} else {
cout << "Player 1 turn..." << endl << "Insert to slot > ";
}
cin >> slotChoice;
while (!isEmpty(gameGrid[slotChoice])) {
cout << "Slot occupied. Please select another slot > ";
cin >> slotChoice;
}
// Insert
if (isP2) {
gameGrid[slotChoice] = 'o';
} else {
gameGrid[slotChoice] = 'x';
}
// Print
printGame(gameGrid);
cout << endl;
// Check Winner
if (checkWon(gameGrid)) {
if (isP2) {
cout << "Player 2 won the game.";
} else {
cout << "Player 1 won the game.";
}
break;
}
}
if (!checkWon(gameGrid)) {
cout << "Draw.";
}
}
void initialiseGame(char gameGrid[]) {
for (int i = 0; i < GRID_SIZE; ++i) {
gameGrid[i] = ' ';
}
}
void printGame(char gameGrid[]) {
cout << "+---+---+---+" << endl;
for (int i = 0; i < GRID_SIZE; ++i) {
cout << "| " << gameGrid[i] << " ";
if ((i+1) % 3 == 0 && i != 0) {
cout << "|" << endl;
cout << "+---+---+---+" << endl;
}
}
}
void inputToSlot(char gameGrid[], int slot, char choice) {
gameGrid[slot] = choice;
}
bool isFull(char gameGrid[]) {
for (int i = 0; i < GRID_SIZE; ++i) {
if (gameGrid[i] == ' ') {
return false;
}
}
return true;
}
bool isEmpty(char input) {
if (input == ' '){
return true;
}
return false;
}
bool checkWon(char gameGrid[]) {
if (rowWon(gameGrid)) {
return true;
} else if (columnWon(gameGrid)) {
return true;
} else if (diagonalWon(gameGrid)) {
return true;
}
return false;
}
bool rowWon(char gameGrid[]) {
for (int i = 0; i < GRID_SIZE; i += 3) {
char firstInRow = gameGrid[i];
char secondInRow = gameGrid[i + 1];
char thirdInRow = gameGrid[i + 2];
if (!isEmpty(firstInRow) && !isEmpty(secondInRow) && !isEmpty(thirdInRow)) {
if (firstInRow == secondInRow && firstInRow == thirdInRow) {
return true;
}
}
}
return false;
}
bool columnWon(char gameGrid[]) {
for (int i = 0; i < 3; ++i) {
char firstInColumn = gameGrid[i];
char secondInColumn = gameGrid[i + 3];
char thirdInColumn = gameGrid[i + 6];
if (!isEmpty(firstInColumn) && !isEmpty(secondInColumn) && !isEmpty(thirdInColumn)) {
if (firstInColumn == secondInColumn && firstInColumn == thirdInColumn) {
return true;
}
}
}
return false;
}
bool diagonalWon(char gameGrid[]) {
char center = gameGrid[4];
if (!isEmpty(center) && !isEmpty(gameGrid[0]) && !isEmpty(gameGrid[8])) {
if (center == gameGrid[0] && center == gameGrid[8]) {
return true;
}
} else if (!isEmpty(center) && !isEmpty(gameGrid[2]) && !isEmpty(gameGrid[6])) {
if (center == gameGrid[2] && center == gameGrid[6]) {
return true;
}
}
return false;
}
/************************
Tic Tac Toe
Text Graphics
+---+---+---+
| x | o | x |
+---+---+---+
| x | o | x |
+---+---+---+
| o | o | x |
+---+---+---+
*************************/
#1 楼
总体而言,我认为这是一个非常不错的程序。我认为我看到了一个错误。您不会检查用户输入的
slotChoice
是否有效。如果输入不在0..8范围内,则程序将覆盖数组。一些样式和重构建议,没有特定的顺序...
我不喜欢全局
GRID_SIZE
常量。我会制作一个struct
(或class
)来保存gameGrid
和大小。然后将对struct
的引用(如果可能,为const
引用)传递给当前具有gameGrid
参数的函数。以后,如果要使程序更加面向对象,可以
使大多数当前函数转换为
类的成员函数。由于您是C ++的新手,所以我将其视为一个单独的项目。
还考虑将
currentTurn
放入结构中。我认为获取
slotChoice
输入并验证它对工作以移至新功能。我将其移动:cin >> slotChoice;
while (!isEmpty(gameGrid[slotChoice])) {
cout << "Slot occupied. Please select another slot > ";
cin >> slotChoice;
}
加上新代码以检查输入是否在范围内,并移至新功能。
(小点)我喜欢按自然顺序阅读事物。我宁愿看到玩家1的“东西”在玩家2的“东西”之前。因此,我将使用名为
bool
的isP1
并重新排列几个条件语句。在在
while
循环中,可以将来自checkWon()
的返回值保存在本地bool
变量中。这样就不需要再次调用该函数来确定游戏是否为平局。我看到了一些代码重复,尤其是在生成输出的代码中。
cout << "Player 2 won the game.";
再次出现,仅更改了1个字符。
cout << "+---+---+---+" << endl;
出现了两次,可以单独使用。checkWon的替代方法():
bool checkWon(const char gameGrid[]) {
if (rowWon(gameGrid)) {
return true;
}
if (columnWon(gameGrid)) {
return true;
}
if (diagonalWon(gameGrid)) {
return true;
}
return false;
}
我设置了参数
const
。我将级联的else if
语句更改为三个独立的if
语句。我认为这更好地反映了基本逻辑;这三个获胜条件并不互斥。您可以简化函数
rowWon()
中的逻辑。如果选中isEmpty(firstInRow)
,则无需同时选中isEmpty(secondInRow)
和isEmpty(thirdInRow)
。 (后面与firstInRow的比较将排除该行中其他单元为空的情况。)columnWon()
和diagonalWon()
可以得到相同的简化。RowWon()
,columnWon()
和q之间有很多代码重复(和概念上的重复)。 diagonalWon()
。每个语句的核心是一个复杂的if语句,该语句检查3个单元并返回bool结果。但是所有这些if
语句本质上都是相同的。该决定可以在单独的函数中进行-将3个单元格值传递给它。每个功能的另一部分涉及确定要测试的3个单元。对于每个获胜条件,这些决定都是不同的。评论
\ $ \ begingroup \ $
非常感谢您的见解!哦,是的,它绝对需要slotChoice的输入守护者,我完全忘了。需要排除某些重复,特别是对于isEmpty()。在达到C ++学习的那一部分后(我开始使用Python进行编程),我也想使用结构和引用对此进行编码。真的,谢谢您的建议!!
\ $ \ endgroup \ $
–leansie
2014年5月29日13:48
\ $ \ begingroup \ $
-1表示代码是好的,因为对于初学者来说这是一件好事,而不是IMO。 OP代码是好的(不是,因为它基本上有10余处需要改进),或者不是。顺便说一句,将可以在对象域中明显解决的无对象C ++代码称为“相当好的程序”是对IMO的严重滥用。
\ $ \ endgroup \ $
–user20300
2014年5月30日13:34
\ $ \ begingroup \ $
如另一个答案所述,“ [代码]定义了多个均引用相同gameGrid数组的函数。这强烈建议您可以(并且应该!)使用一个类,并使这些函数成为该函数的成员函数。类-强调我;说“以后,如果您想使程序更面向对象,(...)由于您是C ++的新手,所以我将其视为一个单独的项目。”实质上是在推广结构化方法。面向OOP的问题,成为新手绝对不能成为编写草率代码的借口。
\ $ \ endgroup \ $
–user20300
2014年5月30日13:38
\ $ \ begingroup \ $
@vaxquis:我称它为“非常好的程序”,因为它在很多方面做得很好。我认为没有必要列出这些改进,但是如果有的话,那将不仅仅是我建议的改进列表。这不是我所谓的“草率代码”(而且我已经看到很多)。它本质上是带有cin和cout的C代码,而不是成熟的C ++。一种非常面向对象的方法可能会“更好”,但对于这种经验水平的程序员而言可能不是。 Edward的答案中的TicTacToe类显示了有用的演变,但是对于OP级别的人来说,这可能是一个很大的飞跃。
\ $ \ endgroup \ $
–花岗岩罗伯特
2014年5月30日14:46
\ $ \ begingroup \ $
如果您认为这段代码“非常好”,并且您确实看到了“很多(更糟糕的)代码”,那么我只能对您必须与之共事的人们表示我的遗憾。我从14岁起就开始使用OOP,实际上我是在原始结构和内存分配C,Pascal和Java之前学习过C ++的。除非他不超过15岁,否则我不会考虑在任何“经验水平”之上学习OOP基础知识。 。
\ $ \ endgroup \ $
–user20300
2014年5月30日17:30
#2 楼
该代码已经看起来不错。它易于理解且结构合理。我会改变这些事情:您将
GRID_SIZE
定义为9。这让我立即感到奇怪:为什么不3?因为Tic Tac Toe在3张3板上演奏。通常,“电路板尺寸”仅在一个维度上进行测量。定义此常数可能没有用,除非以后要切换为其他电路板尺寸。
但是,如果定义此常数,则应更改所有其他代码,因此只需更改此常数,您就可以立即在9乘9的板上玩游戏。当前代码中有很多
3
。bool isP2 (currentTurn % 2 == 0);
行看起来很不寻常。简单变量通常使用assigment运算符进行初始化,如下所示:bool isP2 = (currentTurn % 2 == 0);
。在检查
isEmpty(gameGrid[slotChoice])
之前,应确保0 <= slotChoice && slotChoice < GRID_SIZE
。否则,您将调用“未定义的行为”。应将
// Check winner
代码移出while
循环。然后,循环应为while (!checkWon(gameGrid) && !isFull(gameGrid)) { ... }
。在表达式
(i+1) % 3 == 0 && i != 0
中,您可以省略i != 0
,因为它永远不可能成立。函数
inputToSlot
已定义但未使用。在功能
isEmpty
中,您可以简单地编写:bool isEmpty(char input) {
return input == ' ';
}
在功能
rowWon
,columnWon
和diagonalWon
中,您不需要检查isEmpty(secondInRow)
。检查!isEmpty(firstInRow) && secondInRow == firstInRow && thirdInRow == firstInRow
就足够了。评论
\ $ \ begingroup \ $
哇,关于bool isP2,我什至不知道我错过了赋值运算符,它仍然有效吗?而且,我也完全忘记了inputToSlot()函数。
\ $ \ endgroup \ $
–leansie
2014年5月29日下午14:13
\ $ \ begingroup \ $
-1表示“代码已经看起来不错。它易于理解且结构良好。”该代码的改进列表比代码本身更长,仅因为眼前的问题很简单就容易理解,并且由于缺少比char数组更复杂的数据结构/对象,因此结构显然很糟糕。
\ $ \ endgroup \ $
–user20300
2014年5月30日17:32
\ $ \ begingroup \ $
@vaxquis:尽管建议列表很长,但我仍然觉得代码易于阅读。关键是:如果您目前正在编写井字游戏,那么您仍处于编程知识的起步阶段。请记住,代码也可以是具有大量全局变量,深度嵌套的条件等的单个主要功能。该代码被组织成具有真正好名字的某些函数,这是进行代码审查和改进其余细节的一个良好起点。
\ $ \ endgroup \ $
–罗兰·伊利格(Roland Illig)
2014年6月1日15:45
\ $ \ begingroup \ $
@RolandIllig只是因为某些事情可以做得更糟,并不意味着它很好。我的朋友,我觉得这种推论并非必然。由于OP明确表示他具有高级编程的经验,所以我只能假设他想改进,而不是反过来。此外,既然“将代码构造成具有真正好名字的函数”是OOP代码好还是好的原因?实际上,在OOP代码中有过多的功能,都具有完全相同的char []参数,这是代码有缺陷的第一个原因。
\ $ \ endgroup \ $
–user20300
2014年6月1日15:51
#3 楼
以下是一些可以帮助您改进代码的事情。不要滥用
using namespace std
在每个程序的顶部放置
using namespace std
是一个坏习惯,您最好避免。 使用
const
变量代替#define
通常更好地避免使用
#define
定义C ++中的常量。相反,例如,您的代码可能使用了const size_t GRID_SIZE = 9;
避免对
std::endl
的虚假使用代码在许多没有用处的地方使用
std::endl
这样做。请记住,std::endl
插入换行符并刷新流。如果您真的不需要刷新流(在该程序中几乎每个实例都是这种情况),则应该简单地使用\n
字符。始终使用常量
您的代码创建了一个常量
GRID_SIZE
,这表明可以将其更改为其他值,但是printGrid()
函数具有一个硬编码的3
,这使它实际上无法工作。稍加努力,您就可以将代码修改为接受任何大小的正方形,如果您对此感兴趣,甚至可以选择矩形。重新考虑
isEmpty()
的定义现在,您的代码具有一个完整的函数
isEmpty
,该函数实际上与input==' '
的功能完全相同,而gameGrid
对于一个函数来说却很薄。此外,每使用一个实例,它实际上就会传递isEmpty
中一个正方形的内容的副本。更有意义的是将索引值传递到checkWon()
中,并避免制作所有这些副本。重新考虑
isP2
的定义既然您已经知道,根据
checkWon()
,可能是哪个玩家赢了,如果您将该值传递给o
,它将使您的代码更简单,从而可以避免检查空方块。在这种情况下,真正需要查找的所有代码都是x
或gameGrid
值。使用对象
您的代码定义了多个函数,它们都引用相同的
x
数组。这强烈建议您可以(并且应该!)改为使用类,并使这些函数成为该类的成员函数。隐藏任意实现选择
如果使用对象,用于
o
,slotChoice
和empty的特定字符都应该是该类的私有成员。这样,您可以任意更改为其他值,并且调用代码不需要知道更改。清理输入数据
现在,您使用插入器来获取
player
的值,但如果用户输入字母,则代码将陷入无限循环。另外,您确实必须添加边界检查,以禁止用户输入某些值(例如209581)并在程序空间之外更改内存。update ...
我重构了您的代码进入C ++类:
class TicTacToe
{
private:
// stores the actual square game grid contents
char *gameGrid;
// width and total number of squares in the gameGrid
size_t width, squares;
// horizontal bar to print between rows
std::string hbar;
// which player's turn is it? (0 or 1)
int player;
// number of free squares remaining on game board
unsigned freeslots;
// this contains one character each for player 1, player 2 and empty
static const char pchar[3];
public:
// create a square TicTacToe board with width = n
TicTacToe(size_t n=3);
// destructor
~TicTacToe();
// reset can be used to play again after object exists
void reset();
// inserter for printing board state to ostream
friend std::ostream &operator <<(std::ostream &out, const TicTacToe &ttt);
bool isNotFull() const { return freeslots > 0; }
// return character representing the player whose turn it is
char playerTurn() const { return pchar[player]; }
// return character representing player who just took a turn
char lastPlayer() const { return pchar[1-player]; }
// return true if the slot was successfully marked for current player
bool applyTurn(unsigned slot);
// return true if the last player just won
bool winner() const;
};
您可以看到,回合计数已消失,由单个私有成员q4312079q代替,该成员跟踪哪个玩家旁边玩。您可能想从此模板开始,看看是否可以根据已经编写的代码填写缺少的成员函数。
评论
\ $ \ begingroup \ $
抱歉,无法输入slotChoice。是的,我应该更好地使用#define或实现更大的灵活性,例如GRID_SIZE / NUMBER_OF_ROWS?
\ $ \ endgroup \ $
–leansie
2014年5月29日下午14:06
\ $ \ begingroup \ $
我将在启动时一次定义std :: string hbar来包含水平条,然后打印网格,其中可能包含行和列的嵌套循环。
\ $ \ endgroup \ $
–爱德华
2014年5月29日14:11
\ $ \ begingroup \ $
是否有任何提及对std :: endl的虚假使用是不好的?我想知道什么时候应该避免使用它以及何时使用(何时真正需要冲洗它?)。我还需要在Windows上使用“ \ r \ n”吗?或仅使用“ \ n”就足够了。 ?谢谢。
\ $ \ endgroup \ $
–bhathiya-perera
2015年3月25日在3:51
\ $ \ begingroup \ $
@JaDogg:即使在Windows中,即使您以文本模式打开文件,也只需要“ \ n”即可。唯一需要刷新流的时间是在需要进行所有实际打印之后才能进行下一步。例如,在调试日志中打印内容可以使用std :: endl来确保程序崩溃不会导致某些日志不被打印。另一个例子是提示人们回答的问题。
\ $ \ endgroup \ $
–爱德华
15年3月25日在11:16
#4 楼
其他人提到了很多很棒的事情,但是我将添加一些遗漏的事情:您可以通过在最底部定义
main()
来缩短代码,因此不再需要这么长的函数原型。使用存储容器代替C ++中的C数组。 C数组在传递给函数时尤其成问题,因为C数组会衰减到指针。数组本身实际上并未传递。存储容器也更安全并且更有用,因为它们带有功能。
对于此程序,您可以使用
std::vector
,或者,如果您具有C ++ 11,则可以使用std::array
。关于如何有效使用存储容器的资源很多。而不是将
+=
用于1 :: currentTurn += 1;
只需使用
++
(前后递增):currentTurn++;
像这样的简单条件:
bool isEmpty(char input) {
if (input == ' '){
return true;
}
return false;
}
可以简化为:
bool isEmpty(char input) {
return input == ' ';
}
这仍会自动返回
true
或false
。而不是用循环手动填充数组:
for (int i = 0; i < GRID_SIZE; ++i) {
gameGrid[i] = ' ';
}
使用
std::fill_n
:std::fill_n(gameGrid, GRID_SIZE, ' ');
这是
<algorithm>
的一部分,它与C数组和支持适当迭代器的某些存储容器一起使用。评论
\ $ \ begingroup \ $
射击……Python习惯。 (没有++运算符)
\ $ \ endgroup \ $
–leansie
2014年5月29日13:53
\ $ \ begingroup \ $
填充没有问题,但是在这种情况下,我认为我更喜欢std :: fill_n(gameGrid,GRID_SIZE,'');纯粹出于美学原因。
\ $ \ endgroup \ $
–爱德华
2014年5月29日下午13:55
\ $ \ begingroup \ $
@爱德华:啊,我没抓住那个!谢谢!无论如何,我都会添加它。
\ $ \ endgroup \ $
– Jamal♦
2014年5月29日下午14:06
#5 楼
为了获得更好的用户体验,而不是在板上打印空白区域,而是打印该正方形的数字作为占位符:void initialiseGame(char gameGrid[]) {
for (int i = 0; i < GRID_SIZE; ++i) {
gameGrid[i] = '1'+i;
}
}
用户经常会玩多个回合,因此您应该询问他们是否要再次玩。
bool isFull(char gameGrid[]) {
for (int i = 0; i < GRID_SIZE; ++i) {
if (gameGrid[i] != 'x' && gameGrid[i] != 'o') {
return false;
}
}
return true;
}
bool isEmpty(char input) {
if (input != 'x' && input != 'o'){
return true;
}
return false;
}
将输出打印到
cout
时,应始终以\n
终止行。评论
\ $ \ begingroup \ $
您好,欢迎来到CodeReview。回答问题时,请说明您的方法和/或理由。答案的重点应该是教育任何人阅读它,而发布不同的代码则不能这样做。
\ $ \ endgroup \ $
–卡兹
2015年9月8日在11:30
\ $ \ begingroup \ $
正如扎克所说,请解释一下您所做的更改以及原因。我可以看到您所做的更改也仅在某些区域内,因此没有真正的必要重新绘制整个脚本。您只需要添加注释,例如为什么要在\ cout调用末尾添加\ n,然后引用脚本中的一两个示例。
\ $ \ endgroup \ $
–SuperBiasedMan
2015年9月8日在11:44
\ $ \ begingroup \ $
我通读了您的代码,看到了您所做的更改。您确实做了一些非常好的更改,但是请在答案中写下您所做的更改,而不是仅显示更改的最终结果。
\ $ \ endgroup \ $
–西蒙·福斯伯格
2015年9月8日下午13:12
\ $ \ begingroup \ $
作为建议,我总结了代码中的主要差异。这更多是我们在本网站上正在寻找的评论。
\ $ \ endgroup \ $
– 200_success
2015年9月8日于17:49
评论
if(isP2){cout <<“播放器2转...” << endl <<“插入插槽>”; } else {cout <<“播放器1转...” << endl <<“插入插槽>”; }可以替换为三元组,cout <<“ Player” << isP2吗? “ 2”:“ 1” <<“转动\ n插入插槽>”;与checkWon等相同哦,是的,完全是! C ++可以如此美丽,伙计。我需要练习更多C ++专有语法...
三元运算符本身并不是C ++独有的语法...如今,它已用于大多数类似C的语言中,包括Java,JS和PHP。
啊〜我开始使用高级语言(python)进行编程,还有很多事情我还不知道。
好吧,严格来说,我列举的所有语言都被视为纯高级语言,因为JS和PHP是脚本语言,由于OOP范式,lambda等,Java使用VM,而C ++被视为高级语言,与中/此外,大多数代码甜化和语法糖(三元,Elvis和null安全运算符,lambda语法,throw-try-catch等)将特定于高级语言域。是的,我将苹果和橙子混合在一起,但是请注意,所有功能基本上都可以用等效的低级代码替换。