在C++中连接4

Question

我刚写了一个很简单的游戏--“连接4”。这起作用了--至少我认为是这样--但我想为未来寻求一些建议，并找出什么可以做得更好。我对编程非常陌生，但我想学习编码(我的任务是一年每天工作一个小时)。

#include <iostream>
#include <cstdlib>
#include <unistd.h>

int tab[7][6];
int choice, player;
bool end = false;
int a=0;

void check(int x)
{
    if(tab[x-1][a]!=0 && a<6)
    {
        a++;
        check(x);
    }   else if (player==1 && a<6)
            {
            tab[x-1][a]=1;
            a=0;
            }
        else if (player==2 && a<6)
            {
            tab[x-1][a]=2;
            a=0;
            }
        else
        {
        std::cout << "WRONG!" << std::endl;
        a=0;
        player++;
        }
}

int draw()
{
    system("clear");
    for(int i = 0; i<9; i++)
    {
        if(i<2)
        {
        std::cout<<"-";
        } else if(i>7)
        {
        std::cout<<i-1<<"--"<<std::endl;
        } else {
        std::cout<<i-1<<"----";
        }
    }
    for(int i = 0; i<6; i++)
    {
        for(int j = 0; j<7; j++)
        {
            if(tab[j][i]!=0)
            {
                if(tab[j][i]==1)
                {
                std::cout<<"| X |";
                }else std::cout<<"| O |";
            }
            else std::cout<<"|   |";
        } std::cout<<std::endl;
    }
    for(int i = 0; i<35; i++)
    {
        std::cout<<"=";
    } std::cout<<std::endl;
}

int win_check()
{
    for(int i = 0; i<6; i++)
    {
        for(int j = 0; j<7; j++)
        {
        if(tab[j][i]==1 && tab[j+1][i+1]==1 && tab[j+2][i+2]==1 && tab[j+3][i+3]==1)
            {
            end = true;
            std::cout << "\nPLAYER 1 WIN!" << std::endl;
            }
        if(tab[j][i]==1 && tab[j+1][i-1]==1 && tab[j+2][i-2]==1 && tab[j+3][i-3]==1)
            {
            std::cout << "\nPLAYER 1 WIN!" << std::endl;
            end = true;
            }
        if(tab[j][i]==2 && tab[j+1][i-1]==2 && tab[j+2][i-2]==2 && tab[j+3][i-3]==2)
            {
            std::cout << "\nPLAYER 2 WIN!" << std::endl;
            end=true;
            }
        else if(tab[j][i]==2 && tab[j-1][i-1]==2 && tab[j-2][i-2]==2 && tab[j-3][i-3]==2)
            {
            end = true;
            std::cout << "\nPLAYER 2 WIN!" << std::endl;
            }
        else if(tab[j][i]==1 && tab[j][i-1]==1 && tab[j][i-2]==1 && tab[j][i-3]==1)
            {
            std::cout << "\nPLAYER 1 WIN!" << std::endl;
            end = true;
            }
        else if(tab[j][i]==1 && tab[j-1][i]==1 && tab[j-2][i]==1 && tab[j-3][i]==1)
            {
            std::cout << "\nPLAYER 1 WIN!" << std::endl;
            end = true;
            }
        else if(tab[j][i]==2 && tab[j][i-1]==2 && tab[j][i-2]==2 && tab[j][i-3]==2)
            {
            std::cout << "\nPLAYER 2 WIN!" << std::endl;
            end = true;
            }
        else if(tab[j][i]==2 && tab[j-1][i]==2 && tab[j-2][i]==2 && tab[j-3][i]==2)
            {
            std::cout << "\nPLAYER 2 WIN!" << std::endl;
            end = true;
            }
        }
    }
}
int p_choice()
{
    player = 1;
    while(end!=true)
    {
    std::cout << "PLAYER " << player << ": ";
    std::cin >> choice;
        if (choice>0 && choice<8)
        {
            check(choice);
            draw();
            if (player == 1)
            {
                player++;
            }
            else
            {
                player--;
            }
        }
    else
        {
    std::cout << "WRONG CHOICE!" << std::endl;
        }
    win_check();
    }
}


int main()
{
    system("clear");
    std::cout<<"WELCOME IN CONNECT 4"<<std::endl;
    sleep(1);
    draw();
    p_choice();
    return 0;
}

Answer 1

对于刚开始的程序员来说，这是一个很好的开始。

当然，有很多事情需要评论。我将把重点放在我认为对一个新程序员来说最重要的工作上。

编译器警告

当我编译您的代码时，我会收到三个关于非无效函数的警告，但没有return语句。draw()、win_check()和p_choice()可能都应该被声明为void，而不是int。

一定要打开所有编译器警告，编译器越挑剔越好，并且要注意收到的每一个警告。修复您的代码，这样您就不会收到任何警告。这可能会帮助您避免许多错误。

正如我在上面的评论中提到的，win_check()是不正确的。我将把它作为一个练习留给读者去找出为什么和如何修复它。

名称

在总体上，使用变量名和函数名做得很好。我会选择更加冗长，使用table而不是tab，使用draw_table()而不是draw()。这里的例外是a。作为一个全局变量，您希望通过其名称来明确其含义。我不知道它的作用或意义。

也就是说，它实际上不是一个全局变量，您只在check函数中使用它，并且不需要在函数调用之间存储它，那么为什么不将它作为一个局部变量呢？

全局变量

尽管如此，最好还是避免使用所有的全局变量。例如，可以将draw()声明为

void draw_table(int const table[7][6]);

与其他函数类似，main()“拥有”所有这些变量，并在函数调用中传递它们。请注意，向函数传递如上所述的数组将指针传递给数组，因此该函数可以修改表。我将const添加到函数声明中，以指示draw_table()不修改表。其他修改它的函数将以一个非const数组作为输入。

常数

您的值6和7分散在代码中。将这些变为文件顶部的实际常量：

constexpr int table_width = 7;
constexpr int table_height = 6;

这有两个优点：(1)当您在代码中使用for(int i=0; i<6; i++)时，不清楚6是什么，如果您使用for(int i=0; i<table_height; i++)，而不是立即清楚您在做什么。(2)如果你决定要一个不同大小的桌子，就很容易改变。使用这样的常量将大大减少bug。

统一责任规则

除了draw()之外，您的所有函数都执行多种操作。试着把画面和游戏逻辑分开写成不同的功能。例如，win_check()应该只检查一行中是否有四个相同值的元素。如果有人赢了，它将返回true (不改变任何全局变量，不写入屏幕)。请注意，只有当前的球员才能获胜，S/他没有任何动作可以使另一名球员获胜。然后在p_choice()中的循环中调用win_check()，并对其返回值执行操作：

void p_choice() {
  int table[table_width][table_height]
  player = 1;
  while(true) {
    std::cout << "PLAYER " << player << ": ";
    std::cin >> choice;
    if (choice>0 && choice<8) {
      check(table, choice);
      if (win_check(table)) {
        std::cout << "\nPLAYER " << player << " WINS!\n";
        return;
      }
      draw(table);
      player = player == 1 ? 2 : 1;
    } else {
      std::cout << "WRONG CHOICE!\n";
    }
  }
}

风格一致性

请确保您的缩进是一致的，大括号的位置是一致的，等等。这使得代码更容易阅读，因此您将产生更少的bug，并且您将更容易地找到您所做的bug。

期待您的下一个项目！

Answer 2

假设你能修好你的窃听器。

让我们分析一下你的代码。

#包含 #包括 #包含

我注意到您已经用"linux“标记标记了您的问题，您还使用了#include <unistd.h>。因此，我想您已经知道，您已经将代码与linux紧密结合在一起，移植它将很困难。你说了算。但是，对代码做一些修改并不难，这样它就可以跨平台。它包括将特定于平台的函数( system()和sleep()调用)包装在一个函数中，然后在函数的正文中包含#ifdef/#endif对，为您所处的平台选择合适的代码。

即使您仍然使用Linux，将自己绑定到系统调用也是个坏主意。考虑一下，如果您想修改代码，使其不在终端上运行，会发生什么情况。也许您想让它作为一个web应用程序运行，或者使用Qt或Tk工具包的GUI运行。在代码中加上对cout和system()的调用，这几乎是不可能的。将程序的逻辑与用户交互分开要好得多，这样相同的逻辑就可以很容易地在不同的上下文中使用。

int选项卡7；int选择，player；bool end = false；int a=0；

这里有几个问题。您使用的是全局变量，这意味着逻辑在其他地方很难合并--这些变量可能与您集成的其他代码发生冲突。至少，使用C++类封装您的数据，以便它只在需要的地方可用。如果没有，则在适当的范围内声明变量，并将它们作为参数传递给需要它们的函数。

此外，您还需要更好地命名这些变量。如果tab是game_board或类似的，那么它会更好。player应该是current_player，我不知道a应该是什么。变量名必须是描述性的。

无效检查(Int x)

检查一下？查什么？x在这里到底是什么意思？这个功能到底是做什么的？它是void，因此它显然改变了全局状态，但它的名称与此相冲突。

{if(制表符x-1！=0 && a<6)

这些支票坏了。如果是a>=6，那么第一个数组引用将超出界限。您需要首先检查a。

{ a++; check(x);

此函数不需要是递归的。在几乎所有情况下，递归都是不必要的。用循环代替。编程课程教递归来强迫学生思考，而不是因为它是一种很好的技巧。

} else if (player==1 && a<6) { tab[x-1][a]=1; a=0; } else if (player==2 && a<6) { tab[x-1][a]=2; a=0; }

注意到这两个if子句有什么相似之处吗？它们可以很容易地结合起来。不要重复你自己。计算机善于重复，让它做它擅长的事情。那tab[x-1][a] = player呢？

else { std::cout << "WRONG!" << std::endl; a=0; player++; } }

因此，据我所知，在一个无效的移动，球员被吼，并失去了他们的回合？太刺耳了！人是不完美的人，他们会犯错。为什么要故意对计算机程序进行惩罚？和你的用户一起工作，不要和他们打架。这里还有一个bug，如果玩家2犯了一个错误，那么player变成了3，程序就失败了。

最后一个注意事项:几乎没有必要使用std::endl。输出缓冲区将在正确的时间自动刷新，例如当用户需要提供输入时。自己调用std::endl只会降低性能。坚持"\n"就行了。

int (){system(“清除”)；for(int i= 0；i<9；i++) { if(i<2) {std：：cout<<-“；} if(i>7) {std：：cout<7{std：：cout<

等等等一下。为什么要循环i，然后根据i的值执行不同的操作？这不是循环的目的。只需做单独的代码！循环什么是常见的，并避免i-1类型的构造，他们会搞砸你。

for(int i = 0; i<6; i++) { for(int j = 0; j<7; j++) { if(tab[j][i]!=0) { if(tab[j][i]==1) { std::cout<<"| X |"; }else std::cout<<"| O |"; } else std::cout<<"| |"; } std::cout<<std::endl;

再次查看C.的switch语句，不需要std::endl。

} for(int i = 0; i<35; i++) { std::cout<<"="; } std::cout<<std::endl; } int win\_check() { for(int i = 0; i<6; i++) { for(int j = 0; j<7; j++) { if(tab[j][i]==1 && tab[j+1][i+1]==1 && tab[j+2][i+2]==1 && tab[j+3][i+3]==1) { end = true; std::cout << "\nPLAYER 1 WIN!" << std::endl; } if(tab[j][i]==1 && tab[j+1][i-1]==1 && tab[j+2][i-2]==1 && tab[j+3][i-3]==1) { std::cout << "\nPLAYER 1 WIN!" << std::endl; end = true; } if(tab[j][i]==2 && tab[j+1][i-1]==2 && tab[j+2][i-2]==2 && tab[j+3][i-3]==2) { std::cout << "\nPLAYER 2 WIN!" << std::endl; end=true; } else if(tab[j][i]==2 && tab[j-1][i-1]==2 && tab[j-2][i-2]==2 && tab[j-3][i-3]==2) { end = true; std::cout << "\nPLAYER 2 WIN!" << std::endl; } else if(tab[j][i]==1 && tab[j][i-1]==1 && tab[j][i-2]==1 && tab[j][i-3]==1) { std::cout << "\nPLAYER 1 WIN!" << std::endl; end = true; } else if(tab[j][i]==1 && tab[j-1][i]==1 && tab[j-2][i]==1 && tab[j-3][i]==1) { std::cout << "\nPLAYER 1 WIN!" << std::endl; end = true; } else if(tab[j][i]==2 && tab[j][i-1]==2 && tab[j][i-2]==2 && tab[j][i-3]==2) { std::cout << "\nPLAYER 2 WIN!" << std::endl; end = true; } else if(tab[j][i]==2 && tab[j-1][i]==2 && tab[j-2][i]==2 && tab[j-3][i]==2) { std::cout << "\nPLAYER 2 WIN!" << std::endl; end = true; } } } }

再说一遍这里有很多重复的代码。看看你能不能找出共同点，减少这里的代码数量，利用计算机使用循环轻松重复操作的能力。

int p_choice() { player = 1；while(end!=true)

因为end是bool，所以您可以只使用while (!end)

{ std::cout << "PLAYER " << player << ": "; std::cin >> choice; if (choice>0 && choice<8) { check(choice); draw(); if (player == 1) { player++; } else { player--; }

你知道什么数学运算会改变1到2和2到1？(提示:考虑减法)

} else { std::cout << "WRONG CHOICE!" << std::endl; } win\_check(); } } int main() { system("clear"); std::cout<<"WELCOME IN CONNECT 4"<<std::endl; sleep(1); draw(); p\_choice(); return 0; }

对于sleep()，如果您正在使用C++11，则已经存在跨平台sleep() --参见这个问题。如果您不在C++11上，您可以使用Boost库，即提供跨平台sleep()。，但我怀疑是否需要延迟。这种延迟如何改善用户体验？如果用户启动您的程序，他们真的想在输入之前等待吗？