我一直在教自己如何编程,现在希望对我最近的项目提出一些批评,创建一个Tic Tac Toe程序!我希望人们指出我的代码中的一些缺陷,以及今后应该尝试改进的一些领域。

评论

“ //没有人赢”是无法访问的代码。

#1 楼

好的..我看了看你的代码。让我们一次完成一个步骤...

我正在使用安装了Resharper的VisualStudio。
这会使您的代码很大一部分灰显。

if子句

if (winFlag == false) // No one won ---------------------------
{
  ...
}


这是因为winFlag不能为假。这是因为上面的循环仅在winFlag为true时才退出。

所以...那里存在问题。我们将做到这一点。让我们做一些清理。

CheckWin是一个不错的起点。很多冗长。
需要添加注释说明该方法本身并不是在说话。

    if(Pos[1] == "O" && Pos[5] == "O" && Pos[9] == "O") // Diagonal -----------------------------------------
    {
        return true;
    }


此模式重复出现……正在做什么?检查一行。
为此添加一个方法。

    private static bool IsLine(int index0, int index1, int index2, string piece)
    {
        return Pos[index0] == piece && Pos[index1] == piece && Pos[index2] == piece;
    }


应用该功能,我们得到...大量的重复。所有测试均进行两次,一次使用“ O”,一次使用“ X”。
让我们解决这个问题。...现在我们可以通过两种方式来做到这一点。循环播放可能的片段,或仅更改IsLine以检查行匹配中的所有片段。

让我们尝试第二个。

static bool CheckWin() // Win checker method ================================================
    {
        if (IsLine(1, 2, 3, "O")) // Horizontal ----------------------------------------
        {
            return true;
        }
        if (IsLine(4, 5, 6, "O"))
        {
            return true;
        }
        if (IsLine(7, 8, 9, "O"))
        {
            return true;
        }

        if(IsLine(1, 5, 9, "O")) // Diagonal -----------------------------------------
        {
            return true;
        }
        if(IsLine(7, 5, 3, "O"))
        {
            return true;
        }

        if(IsLine(1, 4, 7, "O"))// Coloumns ------------------------------------------
        {
            return true;
        }
        if(IsLine(2, 5, 8, "O"))
        {
            return true;
        }
        if(IsLine(3, 6, 9, "O"))
        {
            return true;
        }

        if(IsLine(1, 2, 3, "X")) // Horizontal ----------------------------------------
        {
            return true;
        }
        if(IsLine(4, 5, 6, "X"))
        {
            return true;
        }
        if(IsLine(7, 8, 9, "X"))
        {
            return true;
        }

        if(IsLine(1, 5, 9, "X")) // Diagonal -----------------------------------------
        {
            return true;
        }
        if(IsLine(7, 5, 3, "X"))
        {
            return true;
        }

        if(IsLine(1, 4, 7, "X")) // Coloumns ------------------------------------------
        {
            return true;
        }
        if(IsLine(2, 5, 8, "X"))
        {
            return true;
        }
        if(IsLine(3, 6, 9, "X"))
        {
            return true;
        }
        return false;
    }


我们得到:

    private static bool IsAnyLine(int index0, int index1, int index2)
    {
        return IsLine(index0, index1, index2, Pos[index0]);
    }


现在...所有那些“返回真值怎么办?退出此方法。
如果其中任何一种情况为真,我们只需要返回真即可。

    static bool CheckWin() // Win checker method ================================================
    {
        if(IsAnyLine(1, 2, 3)) // Horizontal ----------------------------------------
        {
            return true;
        }
        if(IsAnyLine(4, 5, 6))
        {
            return true;
        }
        if(IsAnyLine(7, 8, 9))
        {
            return true;
        }

        if(IsAnyLine(1, 5, 9)) // Diagonal -----------------------------------------
        {
            return true;
        }
        if(IsAnyLine(7, 5, 3))
        {
            return true;
        }

        if(IsAnyLine(1, 4, 7))// Coloumns ------------------------------------------
        {
            return true;
        }
        if(IsAnyLine(2, 5, 8))
        {
            return true;
        }
        if(IsAnyLine(3, 6, 9))
        {
            return true;
        }

        return false;
    }


哦...“如果X == true返回true,否则返回false”与“ return X”相同。

static bool CheckWin() // Win checker method ================================================
{
    if (IsAnyLine(1, 2, 3) ||
        IsAnyLine(4, 5, 6) ||
        IsAnyLine(7, 8, 9) ||
        IsAnyLine(1, 5, 9) ||
        IsAnyLine(7, 5, 3) ||
        IsAnyLine(1, 4, 7) ||
        IsAnyLine(2, 5, 8) ||
        IsAnyLine(3, 6, 9)) return true;
    return false;
}


现在...有趣的是,水平线都是(x,x + 1,x + 2),所有垂直线都是(x,x + 3,x + 6)。一个对角线是“(x,x + 4,x + 8)”,另一个是(3,3 + 2,3 + 4)

    static bool CheckWin() // Win checker method ================================================
    {
        return IsAnyLine(1, 2, 3) || // Horizontal 
               IsAnyLine(4, 5, 6) || // Horizontal 
               IsAnyLine(7, 8, 9) || // Horizontal 
               IsAnyLine(1, 5, 9) || // Diagonal
               IsAnyLine(7, 5, 3) || // Diagonal
               IsAnyLine(1, 4, 7) || // Vertical
               IsAnyLine(2, 5, 8) || // Vertical
               IsAnyLine(3, 6, 9);   // Vertical
    }


又一件事...

    private static bool IsAnyHorizontalLine(int startindex)
    {
        return IsAnyLine(startindex, startindex + 1, startindex + 2);
    }

    private static bool IsAnyVerticalLine(int startindex)
    {
        return IsAnyLine(startindex, startindex + 3, startindex + 6);
    }

    static bool CheckWin() // Win checker method ================================================
    {
        return IsAnyHorizontalLine(1) ||
               IsAnyHorizontalLine(4) ||
               IsAnyHorizontalLine(7) ||
               IsAnyLine(1, 5, 9) ||
               IsAnyLine(7, 5, 3) ||
               IsAnyVerticalLine(1) ||
               IsAnyVerticalLine(2) ||
               IsAnyVerticalLine(3);
    }


好吧,我们可以在这里做更多...但是我认为这已经足够。

让我们看一下主要逻辑。



现在这是一个重要功能。

让我们介绍一些辅助功能...每次您发表评论时,我都会将其设为功能。与循环的主体相同..使其成为函数。

    private static bool IsAnyLine(int start, int step)
    {
        return IsLine(start, start+step, start+step+step, Pos[start]);
    }

    private static bool IsAnyHorizontalLine(int startindex)
    {
        return IsAnyLine(startindex, 1);
    }

    private static bool IsAnyVerticalLine(int startindex)
    {
        return IsAnyLine(startindex, 3);
    }

    static bool CheckWin() // Win checker method ================================================
    {
        return IsAnyHorizontalLine(1) ||
               IsAnyHorizontalLine(4) ||
               IsAnyHorizontalLine(7) ||
               IsAnyLine(1, 4) ||  // Diagonal 
               IsAnyLine(3, 2) ||  // Diagonal
               IsAnyVerticalLine(1) ||
               IsAnyVerticalLine(2) ||
               IsAnyVerticalLine(3);
    }


这使得前几行...

    private static string[] EnterPlayers()
    {
        Console.WriteLine("Hello! This is Tic Tac Toe. If you don't know the rules then stop being an idiot.");
        Console.WriteLine("What is the name of player 1?");
        var player1 = Console.ReadLine();
        Console.WriteLine("Very good. What is the name of player 2?");
        var player2 = Console.ReadLine();
        Console.WriteLine("Okay good. {0} is O and {1} is X.", player1, player2);
        Console.WriteLine("{0} goes first.", player1);
        Console.ReadLine();
        Console.Clear();
        return new [] {player1, player2};
    }


很好。我想知道播放器数组是否有用?也许...但是暂时限制更改并继续使用播放器变量。

在C ++中,最好在方法开始时声明所有变量。在C#中,我们将它们移动到首次使用它们的位置。尝试为每个变量设置最小范围。考虑到这一点,重用变量也是一个坏主意。每次需要时最好制作一个新的。这样可以解开不应耦合的代码。这使更改代码更加容易。

每当我们要求用户提供数据时,“ choice”变量都会被重用。与'correctInput'相同。

        string[] players = EnterPlayers();
        string player1 = players[0];
        string player2 = players[1];


总是相同的代码,所以让我们做一个函数。

while (correctInput == false)
{
     Console.WriteLine("Which position would you like to take?");
     choice = int.Parse(Console.ReadLine());
     if (choice > 0 && choice < 10)
     {
         correctInput = true;
     }
 }


或更简单...

private static int AskTheUser(string prompt)
{
    bool correctInput = false;
    int choice = 0;
    while (correctInput == false)
    {
        Console.WriteLine(prompt);
        choice = int.Parse(Console.ReadLine());

        if (choice > 0 && choice < 3)
        {
            correctInput = true;
        }
    }
    return choice;
}


该方法仍然太大,无法阅读...让我们抽出“检查是否已经采取了斑点”

private static int AskTheUser(string prompt, int min, int max)
{
    while (true)
    {
        Console.WriteLine(prompt);
        int choice = int.Parse(Console.ReadLine());

        if (choice >= min && choice <= max)
        {
            return choice;
        }
    }
}
... 
var choice = AskTheUser("Enter your option: ", 1, 2);


“您现在想做什么?

    private static bool TryToPlacePiece(int move, string opponentsPiece, string piece)
    {
        if (Pos[move] == opponentsPiece) // Checks to see if spot is taken already --------------------
        {
            Console.WriteLine("You can't steal positions asshole! ");
            Console.Write("Try again.");
            Console.ReadLine();
            Console.Clear();
            return true;
        }
        Pos[move] = piece;
        return false;
    }


我在这里做了一些代码操作...但是现在,它是一种返回true继续播放的方法,否则返回false。





每转两圈,逻辑不同。如果我们为循环提取一个方法,我们可以看到它需要什么参数

    private static bool AskToPlayAgain()
    {
        Console.WriteLine("What would you like to do now?");
        Console.WriteLine("1. Play again");
        Console.WriteLine("2. Leave");

        var choice = AskTheUser("Enter your option: ", 1, 2);

        Console.Clear();
        if (choice == 1) return true;

        Console.WriteLine("Thanks for playing!");
        Console.ReadLine();
        return false;
    }


让我们看一下“ TakeTurn” ...

            while (winFlag == false) // Game loop ------------------------------------------------------
            {
                if (TakeTurn(player1, score1, player2, score2, turn)) continue;

                winFlag = CheckWin();

                if (winFlag == false)
                {
                    if (turn == 1)
                    {
                        turn = 2;
                    }
                    else if (turn == 2)
                    {
                        turn = 1;
                    }
                    Console.Clear();
                }
            }


唯一使用'score1'和'score2'的时间是前几行。让我们把那些拉出

q.4312078q

..让我们将'turn == 1'逻辑分组。

    private static bool TakeTurn(string player1, int score1, string player2, int score2, int turn)
    {
        DrawBoard();
        Console.WriteLine("");

        Console.WriteLine("Score: {0} - {1}     {2} - {3}", player1, score1, player2, score2);
        if (turn == 1)
        {
            Console.WriteLine("{0}'s (O) turn", player1);
        }
        if (turn == 2)
        {
            Console.WriteLine("{0}'s (X) turn", player2);
        }

        var move = AskTheUser("Which position would you like to take?", 1, 9);

        if (turn == 1)
        {
            if (TryToPlacePiece(move, "X", "O")) return true;
        }
        if (turn == 2)
        {
            if (TryToPlacePiece(move, "O", "X")) return true;
        }
        return false;
    }


拉出方法

    private static void RefreshBoard(string player1, int score1, string player2, int score2)
    {
        DrawBoard();
        Console.WriteLine("");

        Console.WriteLine("Score: {0} - {1}     {2} - {3}", player1, score1, player2, score2);
    }


这看起来很像应该放在TryToPlacePiece中。

所以我们得到...

    private static bool TakeTurn(string player1, string player2, int turn)
    {
        if (turn == 1)
        {
            Console.WriteLine("{0}'s ({1}) turn", player1, "O");
            var move = AskTheUser("Which position would you like to take?", 1, 9);
            if (TryToPlacePiece(move, "X", "O")) return true;
        }
        else
        {
            Console.WriteLine("{0}'s ({1}) turn", player2, "X");
            var move = AskTheUser("Which position would you like to take?", 1, 9);
            if (TryToPlacePiece(move, "O", "X")) return true;
        }
        return false;
    }


现在Main是可读的。。。:a


'CheckWin'设置了'winFlag'。但是CheckWin不会检测到平局。如果没有更多可放置的内容,则达到平局。 (或9次移动后)

“ TryToPlaceAPiece”也有问题。它会阻止您将一个棋子放在对手的棋子之上,而不是放在自己的棋子之上。

如果这个“转弯”变量留下了最大的气味。许多事情都将其用作指示哪个玩家活跃的指标。为什么其余代码会关心哪个播放器处于活动状态?两者的逻辑应该相同。让我们将所有玩家特定的逻辑分组在一起。首先,将其全部移出main方法,然后移入辅助方法。

我们可以使用players数组,我将创建一个乐谱数组并将各个片段放入数组中...然后我们得到
br />这些有用的方法。

好吧,这太长了,让我们跳过一些步骤。主要看起来像现在。
我们将'turn-1'用作玩家数组,分数数组和棋子数组中的玩家索引。我们不妨将转弯更改为从0开始,以删除逻辑。

所以...这是最终的代码...总共

    private static bool TakeATurn(string player, string playerPiece, string opponentsPiece)
    {
        Console.WriteLine("{0}'s ({1}) turn", player, playerPiece);
        var move = AskTheUser("Which position would you like to take?", 1, 9);
        return TryToPlacePiece(move, opponentsPiece, playerPiece);
    }


这里的目标是使代码更具可读性。您想从“没有明显的错误”变为“显然没有错”。拥有可以显示您的意图并封装实现决策的小型方法可以帮助您实现这一目标。

所以....再次使用斜视测试...最大的方法在某种程度上是可管理的。 >
C#是面向对象的。您可以使用这些功能将代码分成多个块,每个块都有自己的职责。

一些示例:一个知道如何绘制自身和检测线条的Board对象。知道分数,名称和使用的棋子的Player对象。
知道游戏流程的Game对象。
知道人们是否愿意继续比赛的比赛对象播放。

如果您需要更多反馈,我们可以尝试一下。

祝你好运!

评论


\ $ \ begingroup \ $
哇,谢谢你这么有力的回答。非常有见地!我会尝试一下 。
\ $ \ endgroup \ $
–Yummy275
17年8月12日在2:14

\ $ \ begingroup \ $
它的良好反馈值得一提。您没有利用任何OOP主体。没有玩家,游戏,国家的对象..没有
\ $ \ endgroup \ $
–downrep_nation
17年8月13日在13:26

\ $ \ begingroup \ $
@downrep_nation参见答案的最后10行:)
\ $ \ endgroup \ $
–奈杰尔·索恩(Nigel Thorne)
17年8月13日在13:46



#2 楼

错误
同一位玩家可以将其标牌放置在(他已经准备好的)所有位置。尽管您在这里使用了不恰当的语言,但您还是为对手辩护。
您应该检查一下网格中的位置是否已经准备好,而无需检查谁在玩。
我建议您使用一种方法像
private static bool IsPositionTaken(int choice)
{
    return pos[choice] == "X" || pos[choice] == "O";
}  

,现在您可以通过
if (turn == 1)
{
    if (IsPositionTaken(choice)) 
    {
        Console.WriteLine("This position is allready taken! ");
        Console.Write("Try again.");
        Console.ReadLine();
        Console.Clear();
        continue;
    }
    else
    {
        pos[choice] = "O";
    }
}

进行检查,或者是否要引入包含OX的小词典,例如
private static Dictionary<int, string> signs = new Dictionary<int, string>
{
    {1, "O"},
    {2, "X"}
};  

那么这个

if (turn == 1)
{
    if (pos[choice] == "X") // Checks to see if spot is taken already --------------------
    {
        Console.WriteLine("You can't steal positions asshole! ");
        Console.Write("Try again.");
        Console.ReadLine();
        Console.Clear();
        continue;
    }
    else
    {
        pos[choice] = "O";
    }
}
if (turn == 2)
{
    if (pos[choice] == "O") // Checks to see if spot is taken already -------------------
    {
        Console.WriteLine("You can't steal positions asshole! ");
        Console.Write("Try again.");
        Console.ReadLine();
        Console.Clear();
        continue;
    }
    else
    {
        pos[choice] = "X";
    }
}  


会变成这个
if (IsPositionTaken(choice)) 
{
    Console.WriteLine("This position is allready taken! ");
    Console.Write("Try again.");
    Console.ReadLine();
    Console.Clear();
    continue;
}
else
{
    pos[choice] = signs[turn];
}


#3 楼

首先,为什么pos有10个元素,并且您到处都是从1到9?让这个数组有9个元素,从0开始计数。


必须重写CheckWin方法。

定义所有获胜职位的列表:

// List of 3-elements arrays that define indices
// of winning lines (horizontal, vertical, diagonal)
private static readonly int[][] _winningPositions = new[]
{
    // Horizontal lines

    new[] { 0, 1, 2 },
    new[] { 3, 4, 5 },
    new[] { 6, 7, 8 },

    // Vertical lines

    new[] { 0, 3, 6 },
    new[] { 1, 4, 7 },
    new[] { 2, 5, 8 },

    // Diagonal lines

    new[] { 0, 4, 8 },
    new[] { 2, 4, 6 }
}


然后CheckWin将(使用LINQ和lambdas)

static bool CheckWin()
{
    // Check whether there is a line where all symbols are equal
    return _winningPositions.Any(p =>
           {
               var values = p.Select(i => pos[i]) // get symbols on a line
                             .Distinct()          // get unique symbols on a line
                             .ToArray();
               return values.Length == 1 // all symbols are equal
                                         // so count of unique ones is 1
                      && !string.IsNullOrEmpty(values[0]);
           });
}



您不需要在条件中编写布尔文字。代替


while (playing == true)
{
    while (winFlag == false)





while (playing)
{
    while (!winFlag)


评论


\ $ \ begingroup \ $
也许可以更好地解释Lambda在做什么?这被标记为“初学者”,因此他们以前可能从未遇到过Lambda。
\ $ \ endgroup \ $
–帝国查士丁尼
17年8月4日在10:33

\ $ \ begingroup \ $
当我还是初学者时,我只是在搜索所有未知的构造。我认为您自己进行一些研究几乎总是更好。通常,如果某些事情足以解释完成任务的动机,那么就没有动力做进一步的工作了:)另外,我假设如果用户在这里提出问题,他已经知道编程语言的基础知识。
\ $ \ endgroup \ $
– Maxim
17年8月4日在10:43

\ $ \ begingroup \ $
@Maxim至少提及lambda一词可能还是有帮助的(尽管我对Googling的前两个结果=> JavaScript箭头函数和C#lambdas感到有些惊讶,但我希望它会成为很难搜索结构)。
\ $ \ endgroup \ $
– TripeHound
17年8月4日在11:38

\ $ \ begingroup \ $
好,我添加了一些评论:)
\ $ \ endgroup \ $
– Maxim
17年8月4日在12:15

\ $ \ begingroup \ $
+1解释lambda。我以前使用过LINQ(虽然是完整的C#菜鸟),但我不知道从哪里开始重用这段代码。其他一切都非常合理:)。尽管为某人做所有事情可能会降低他们进行研究的意愿,但我也倾向于发现,更有动力的程序员会理解第一个示例(如果得到充分解释),并将其用作深入学习的理由。
\ $ \ endgroup \ $
–布兰登·巴尼(Brandon Barney)
17年8月4日在17:37

#4 楼

您的“ DrawBoard”方法可以得到改进。

static void DrawBoard() // Draw board method ==========================================
{
    Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[1], pos[2], pos[3]);
    Console.WriteLine("-------------------");
    Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[4], pos[5], pos[6]);
    Console.WriteLine("-------------------");
    Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[7], pos[8], pos[9]);
}


“ {0} | {1} | {2}”和“ -------- -----------“被重复多次,因此它们可以并且应该存储在变量中,以便于管理。我了解这是一个很小的项目,但是您当前的使用习惯是一个坏习惯。

static void DrawBoard() // Draw board method ==========================================
{
    string format = "   {0}  |  {1}  |  {2}   ";
    string separator = "-------------------"
    Console.WriteLine(format , pos[1], pos[2], pos[3]);
    Console.WriteLine(separator);
    Console.WriteLine(format , pos[4], pos[5], pos[6]);
    Console.WriteLine(separator);
    Console.WriteLine(format , pos[7], pos[8], pos[9]);
}


通过此更改,如果您想更改板的外观(对于例如,当分隔符与“ |”对齐时,在分隔符中添加“ +”,则只需执行一次即可。您可以在DrawBoard方法中在格式和分隔符之间进行选择。您可以使用For循环来创建板,从而更加灵活。

当前情况下,值i + X是硬编码的,并且不灵活。要解决此问题,可以将Array发送到WriteLine函数以获取参数。

快要完成了!您可能已经注意到,该格式可以自动执行。格式,例如5x5尺寸的木板(如果您选择这样做)变化最小。事实证明,这种增加的灵活性在不同情况下或决定添加不同电路板格式时很有用。修改后,更改变得容易得多,而人为错误也较少。

评论


\ $ \ begingroup \ $
要清理pos + X值,您只需要花一秒钟的for循环,然后就可以很容易地按比例放大(也许甚至允许用户定义板尺寸X *Y。它将添加的代码行值得IMO :)。
\ $ \ endgroup \ $
–布兰登·巴尼(Brandon Barney)
17年8月4日在17:41

\ $ \ begingroup \ $
确实,几行代码。但是,我认为重新实现此功能的目的不是必需的,因为这并不是我最初想要演示的。但是,为完成起见,我将其添加。
\ $ \ endgroup \ $
–Yousend
17年8月4日在17:57

\ $ \ begingroup \ $
@BrandonBarney创建板的循环会很不错。这样,列数将决定格式。
\ $ \ endgroup \ $
–Yousend
17年8月4日在18:08

\ $ \ begingroup \ $
在我看来(来自VBA)我会做一个Square板(由i到ColLength,y到ColLength循环)或Rectangle板(由i到ColLength,y到RowLength循环),然后执行实际位置由pos [y]绘制,而out循环则绘制分隔符。我的C#知识有限,所以我不知道与此有关的正确代码(或反模式)。
\ $ \ endgroup \ $
–布兰登·巴尼(Brandon Barney)
17年8月4日在18:15

\ $ \ begingroup \ $
@BrandonBarney我选择使用单个循环并根据当前行跳到数组中的位置。当我可以避免嵌套循环时,我宁愿没有嵌套循环。对我来说,这是很清楚和简洁的。假设“ pos”数组的长度为colLength * rowLength(+1,因为他在开始时使用0),则此实现不会超出范围。这可能是一个问题,但这不是我要在此答案中演示的内容。我试图证明他可以通过重用变量和循环来减少硬编码值的数量。
\ $ \ endgroup \ $
–Yousend
17年8月4日在18:34

#5 楼

我喜欢Maxim的检查游戏获胜状态的方式,所以这是我的看法(从OP的代码开始,我还添加了一些颜色):

using System;
using System.Linq;

namespace TicTacToeConsole
{
class Program
{
    private enum Turn
    {
        Player0,
        PlayerX
    }
    private enum GameState
    {
        X,
        O,
        Tied,
        NotDone
    }

    static string[] _pos = new string[9] { "0", "1", "2", "3", "4", "5", "6", "7", "8" }; // Array that contains board positions --------------------------------
    public static string NamePlayerO { get; set; }
    public static string NamePlayerX { get; set; }

    public static int ScorePlayer1 { get; set; }
    public static int ScorePlayer2 { get; set; }

    static GameState _gameRes = GameState.NotDone;

    static void Main(string[] args) // Main ==============================================
    {
        int choice;
        var turn = Turn.Player0;

        GameIntro();

        do
        {
            while (_gameRes == GameState.NotDone) // Game loop ------------------------------------------------------
            {
                DrawBoard();
                Console.WriteLine($"{Environment.NewLine}Score: {NamePlayerO} - {ScorePlayer1}     {NamePlayerX} - {ScorePlayer2}");
                Console.WriteLine($"{(turn == Turn.Player0 ? NamePlayerO : NamePlayerX)}'s turn");

                choice = GetPlayerNextPosition();
                if (_pos[choice] == choice.ToString()) // Checks to see if spot is taken already --------------------
                {
                    _pos[choice] = turn == Turn.Player0 ? "O" : "X";
                }
                else
                {
                    Console.WriteLine("Position already taken! Try again");
                    Console.ReadLine();
                    Console.Clear();
                    continue;
                }

                turn = turn == Turn.Player0 ? Turn.PlayerX : Turn.Player0;
                Console.Clear();
                _gameRes = CheckWin();
            }

            // Game Done
            DrawBoard();

            switch (_gameRes)
            {
                case GameState.X:
                    Console.WriteLine($"Player {NamePlayerX} won!");
                    ScorePlayer2++;
                    break;
                case GameState.O:
                    Console.WriteLine($"Player {NamePlayerO} won!");
                    ScorePlayer1++;
                    break;
                case GameState.Tied:
                    Console.WriteLine("It's a draw!");
                    break;
            }
        }
        while (PleaseMakeChoice());
    }

    static int GetPlayerNextPosition()
    {
        int choice;
        while (true)
        {
            Console.WriteLine("Which position would you like to take?");
            if (int.TryParse(Console.ReadLine(), out choice))
                if (choice >= 0 && choice <= 8) break;
                else Console.WriteLine("Not a valid positon! Try again!");
        }
        return choice;
    }

    static bool PleaseMakeChoice()
    {
        bool playing = true;
        int choice;

        Console.WriteLine("Score: {0} - {1}     {2} - {3}", NamePlayerO, ScorePlayer1, NamePlayerX, ScorePlayer2);
        Console.WriteLine("");
        Console.WriteLine("What would you like to do now?");
        Console.WriteLine("1. Play again");
        Console.WriteLine("2. Leave");
        Console.WriteLine("");

        while (true)
        {
            Console.WriteLine("Enter your option: ");
            if (int.TryParse(Console.ReadLine(), out choice))
                if (choice == 1 || choice == 2) break;
        }

        switch (choice)
        {
            case 1:
                _pos = _pos.Select((x, i) => _pos[i] = i.ToString()).ToArray(); // Resets board ------------------------
                Console.Clear();
                _gameRes = GameState.NotDone;
                break;
            case 2:
                Console.Clear();
                Console.WriteLine("Thanks for playing!");
                Console.ReadLine();
                playing = false;
                break;
        }

        return playing;
    }

    static bool CanStillPlay()
    {
        return _pos.Where(x => x == "X" || x == "O").Count() < _pos.Length;
    }

    static void GameIntro()
    {
        Console.WriteLine("Hello! This is Tic Tac Toe.");
        Console.WriteLine("What is the name of player 1?");
        NamePlayerO = Console.ReadLine();
        Console.WriteLine("Very good. What is the name of player 2?");
        NamePlayerX = Console.ReadLine();
        Console.WriteLine("Okay good. {0} is O and {1} is X.", NamePlayerO, NamePlayerX);
        Console.WriteLine("{0} goes first.", NamePlayerO);
        Console.ReadLine();
        Console.Clear();
    }

    static readonly int[][] _winPos = new[]
    {
        new[] {0, 1, 2},
        new[] {3, 4, 5},
        new[] {6, 7, 8},
        new[] {0, 3, 6},
        new[] {1, 4, 7},
        new[] {2, 5, 8},
        new[] {0, 4, 8},
        new[] {2, 4, 6}
    };
    static GameState CheckWin()
    {
        var wonBy = _winPos.Select(x => x.Select(i => _pos[i]).Distinct())
                    .Where(x => x.Count() == 1).ToList();

        if (wonBy.Count() == 0)
            return CanStillPlay() ? GameState.NotDone : GameState.Tied;
        else
            return wonBy.SingleOrDefault().SingleOrDefault() == "X" ? GameState.X : GameState.O;
    }

    static void DrawBoard() // Draw board method ==========================================
    {
        DrawPosLeft(_pos[0]);
        DrawPosMiddle(_pos[1]);
        DrawPosRight(_pos[2]);
        Console.ResetColor();
        Console.WriteLine("-------------------");
        DrawPosLeft(_pos[3]);
        DrawPosMiddle(_pos[4]);
        DrawPosRight(_pos[5]);
        Console.ResetColor();
        Console.WriteLine("-------------------");
        DrawPosLeft(_pos[6]);
        DrawPosMiddle(_pos[7]);
        DrawPosRight(_pos[8]);
        Console.ResetColor();
        Console.WriteLine("-------------------");
    }
    static void DrawPosLeft(string pos)
    {
        SetConsoleColor(pos);
        Console.Write("   {0}", pos);
        Console.ResetColor();
        Console.Write("  |  ");
    }
    static void DrawPosMiddle(string pos)
    {
        SetConsoleColor(pos);
        Console.Write("{0}", pos);
        Console.ResetColor();
        Console.Write("  |  ");
    }
    static void DrawPosRight(string pos)
    {
        SetConsoleColor(pos);
        Console.WriteLine("{0}   ", pos);
        Console.ResetColor();
    }
    static void SetConsoleColor(string pos)
    {
        switch (pos)
        {
            case "O":
                Console.ForegroundColor = ConsoleColor.Red;
                break;
            case "X":
                Console.ForegroundColor = ConsoleColor.Magenta;
                break;
            default:
                Console.ForegroundColor = ConsoleColor.Green;
                break;
        }
    }
}
}


这是我以这种方式编写的一些原因:


我喜欢enum s。当我阅读GameState.NotDone时,我发现它方便易懂
注意,_pos数组现在是从零开始的。我不满意不使用第一个索引位置的想法。

DrawBoard添加了颜色。完全没有必要,除了看起来还不错,什么也没改善。
工作被分成了多种方法,时间不长。我认为不跨越几页的方法会更容易理解。
使用?运算符将回合从一个玩家切换到另一个玩家。它比if else语句(一旦您习惯了)更好。
使用了C#7功能,称为字符串插值。它比string.Format("{0} bla bla {1}", arg1, arg2)的旧方法更容易写出字符串。
请注意如何在主游戏循环中忽略细节。该实现隐藏在其他方法之后。 (例如GetPlayerNextPositionCheckWinDrawBoard)。这也是在OP的代码中完成的。很好,我只是更进一步了一些,以使主循环更短,从而使其更具可读性。
我使用int.TryParse而不是int.Parse。更强大!有时候应该尝试一下。

CheckWin很不错,但很难理解。这是别人的一个聪明主意。我刚刚做到了。


评论


\ $ \ begingroup \ $
我认为为同一问题提供另一种解决方案会使OP拥有不同的观点。我通过看别人的代码来学习。 OP解决问题的一种方式。这是另一个这是一种审查形式。我没有说“应该使用枚举”,而是展示了如何在OP已经理解并知道如何解决的问题中使用它们。我也从Maxim的获胜方式中学到了东西。以为,贡献会很好。
\ $ \ endgroup \ $
–tzup
17年8月7日在7:59

#6 楼

您有一些很好的直觉。

代码应该告诉您3件事:
*它在做什么。
*它是如何工作的。
*为什么要这样做。

您可以使用代码中的不同结构来解决这些问题。


方法名称解释“您在做什么”
方法正文解释“您在做什么”这样做”。您的动机是注释的目的。

在学习编码时,所有代码示例都使用注释来解释“正在发生的事情”,这是不好的做法。每当您觉得需要评论时,这都是很好的反馈。写下评论,然后说“这是什么?怎么做?还是为什么?”

检查您的代码。这些评论大多数都指出了新方法应该在哪里。 :)试试吧!

#7 楼

首先让我惊讶的是,您假设玩家输入了一个数字。永远不要信任用户。在您看来合乎逻辑的并不总是用户会想到的。如果传递给它的字符串实际上不是数字,则Int.Parse将引发异常。我将更改以下代码:

while (correctInput == false)
            {
                Console.WriteLine("Which position would you like to take?");
                choice = int.Parse(Console.ReadLine());
                if (choice > 0 && choice < 10)
                {
                    correctInput = true;
                }
                else
                {
                    continue;
                }
            }


我将其更改为以下内容:

while (correctInput == false)
{
    try
    {
        Console.WriteLine("Which position would you like to take?");
        choice = int.Parse(Console.ReadLine());
        if (choice > 0 && choice < 10)
        {
            correctInput = true;
        }

    }
    catch
    {
        Console.WriteLine("That is not a number"); 
    }
}


首先,删除继续。它什么也不做,因为没有它循环将继续。继续只能在循环的开始或中间使用,而不能在结束时使用。

下一步是尝试。如果int.Parse失败是由于用户恶意或由于手指输入错误而输入了错误的字符串,则这将防止程序因未捕获的异常而崩溃。

最后,我建议分解Main函数,主要是为了提高可读性和理解力。一个好的基准是,如果您的函数长度超过100行,则应该将其拆分为较小的函数。玩游戏的代码实际上不应与胜利文本具有相同的功能。将游戏本身拆分为自己的功能会提高可读性,尤其是如果您希望以后返回并阅读代码时,尤其如此。

评论


\ $ \ begingroup \ $
TryParse()比try..catch要好得多
\ $ \ endgroup \ $
– Heslacher
17年8月4日在10:08

\ $ \ begingroup \ $
@Heslacher是的,但这已被标记为初学者。我个人认为try..catch比out关键字更容易理解。另外,关于try..catch的知识比out关键字有用的更多,因为它的用法更多。
\ $ \ endgroup \ $
–帝国查士丁尼
17年8月4日在10:24

\ $ \ begingroup \ $
对初学者来说是正确的,但对于初学者来说,最好的例外是永远不会发生的例外。
\ $ \ endgroup \ $
– Heslacher
17年8月4日在10:28



\ $ \ begingroup \ $
@Heslacher刚说完一句话,初学者就可以认为使用返回码比使用异常要好:)但是C#并非C,异常是一种很不错的错误处理机制,因此不应忽略。在这种情况下,TryParse显然更好,但在实际应用中,有很多情况下,异常是处理错误的最佳方法。我了解您知道这一点,因此我为初学者撰写了此评论。
\ $ \ endgroup \ $
– Maxim
17年8月4日在10:52

\ $ \ begingroup \ $
我宁愿教初学者int.TryParse,也不愿教他们抓鱼,无所事事。
\ $ \ endgroup \ $
–里克·戴文(Rick Davin)
17年8月4日在12:09

#8 楼

从Program.cs和main中获取代码。我要对您的代码做的第一件事是不要在程序中放入任何类型的代码。我并不是要有0个代码,但它应该只负责1件事:程序状态。这意味着只有足够的代码可以启动,也有足够的退出(可以选择报告异常退出)。在此程度上,让我们迈出第一步。复制整个DrawBoard方法并将其粘贴到新类中。我选择了GameGraphics.cs。现在,在Program.cs中,您需要创建GameGraphic的实例才能使用它。但是在开始之前,我想向您介绍另一种类型的课程。一个测试类。

单元测试

有许多不同的TestFrameworks可用,因此您可以自己搜索它们(NUnit,xUnit,msTest等)。即使我是NUnit团队的一员,我还是建议您坚持使用Visual Studio内置的内容,这样您就不必了解NuGet及其带来的所有好处(尽管这也是一项很好的简单研究项目)。因此,您已经创建了一个新的测试项目(如何创建单元测试项目)。现在让我们为GameGrahpics编写一个测试。可是等等!它不会编译!即使它做到了,也不会自动进行简单的测试。牢骚..确定不是一个大问题。确实,我们只有1个需要编写的测试,我们可以编写更多内容,并且可以做任何事,但是我将编写我的测试以使GameGraphics能够按我希望的方式工作。就是这样:

public class GameGraphics
{
    public void DrawBoard()
    {
        Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[1], pos[2], pos[3]);
        Console.WriteLine("-------------------");
        Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[4], pos[5], pos[6]);
        Console.WriteLine("-------------------");
        Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[7], pos[8], pos[9]);
    }
}


(请注意,您将要重命名该方法和类,但是现在内容很重要)。现在,您将有2个编译错误。首先是在GameGraphics中它不知道名为pos的变量,而在UnitTest1中它将抱怨DrawBoard没有采用string[]的方法。简单修复。在DrawBoard中添加一个名为pos的string[]。现在,您将看到UnitTest1抱怨DrawBoard不返回字符串。再次轻松修复。这是我现在拥有的:

[TestClass]
public class UnitTest1
{
    [TestMethod]
    public void TestMethod1()
    {
        string[] gamePosistion = { "0", "X", "O", "X", "O", "5", "O", "X", "O", "X" };
        string expectedGameboard = GetExpectedGameboard();
        GameGraphics graphics = new GameGraphics();

        string drawnGameBoard = graphics.DrawBoard(gamePosistion);

        Assert.AreEqual(expectedGameboard, drawnGameBoard);
    }

    private static string GetExpectedGameboard()
    {
        StringBuilder sb = new StringBuilder();
        sb.AppendLine(" X | O | X");
        sb.AppendLine(" O | 5 | O");
        sb.AppendLine(" X | O | X");
        string expectedGameboard = sb.ToString();
        return expectedGameboard;
    }
}


我将把它留给您进行测试。如果您遇到困难,上面显示的UnitTest中会有一个很大的提示。我喜欢单元测试的是,它们测试速度很快,并且可以很容易地告诉您所有测试是否通过。例如,如果您运行测试,则测试应显示以下内容:

public class GameGraphics
{
    public string DrawBoard(string[] pos)
    {
        Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[1], pos[2], pos[3]);
        Console.WriteLine("-------------------");
        Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[4], pos[5], pos[6]);
        Console.WriteLine("-------------------");
        Console.WriteLine("   {0}  |  {1}  |  {2}   ", pos[7], pos[8], pos[9]);

        return "";
    }
}


请注意运行速度有多快? Test Duration: 0:00:00.0339269

一旦通过测试,您将可以在GameGraphics的主要方法中创建一个新实例,并将所有对DrawBoard的调用替换为新测试的方法。关于静态的简要说明:我制作了GameGraphics的新实例,并将DrawBoard设为非静态。我这样做是因为,当一个类中有许多静态方法时,这是检查是否可以创建新类的好方法。类可以很简单(应该是)。当您看到自己应该问自己是否对________负责吗?

祝你好运

评论


\ $ \ begingroup \ $
3赞一下标题:“从Program.cs和main中获取代码”。这是“单一责任原则黄砖之路”的第一步。
\ $ \ endgroup \ $
– radarbob
17年8月8日在23:04