[.net] Critique needed on first C# game

Started by
2 comments, last by BradSnobar 17 years, 10 months ago
I finished my first game in C#. Its a terrible console Tic-Tac-Toe clone. I was just wondering if you guys could critique my code and give me some pointers as well. Also I had a few questions. Is there a Console function for clearing the screen and is there a way to change the color of the text? Thanks in advance.

using System;
using System.Collections.Generic;
using System.Text;

namespace TicTacToe
{
    class Program
    {
        static void Main(string[] args)
        {
            char block1 = '1';
            char block2 = '2';
            char block3 = '3';
            char block4 = '4';
            char block5 = '5';
            char block6 = '6';
            char block7 = '7';
            char block8 = '8';
            char block9 = '9';
            char playAgain = 'Y';
            char validChoice = 'N';
            char winState = 'N';
            //int choice;

            while (playAgain == 'Y' || playAgain == 'y')
            {
                //displayBoard(block1, block2, block3, block4, block5, block6, block7, block8, block9);
                Console.WriteLine("Welcome to Tic-Tac-Toe");
                Console.WriteLine("This game is played by choosing the number in the corresponding box.");

                for (int x = 0; x < 10; x++)
                {
                    displayBoard(block1, block2, block3, block4, block5, block6, block7, block8, block9);

                    Console.Write("Please choose your move: ");
                    string stringChoice = Console.ReadLine();
                    int choice = Convert.ToInt32(stringChoice);

                    while (validChoice == 'N')
                    {
                        if (choice == 1)
                        {
                            block1 = 'X';
                            block8 = 'O';
                            validChoice = 'Y';

                            
                        }
                        if (choice == 2)
                        {
                            block2 = 'X';
                            block7 = 'O';
                            validChoice = 'Y';
                            
                        }
                        if (choice == 3)
                        {
                            block3 = 'X';
                            block6 = 'O';
                            validChoice = 'Y';
                            
                        }
                        if (choice == 4)
                        {
                            block4 = 'X';
                            block5 = 'O';
                            validChoice = 'Y';
                            
                        }
                        if (choice == 5)
                        {
                            block5 = 'X';
                            block4 = 'O';
                            validChoice = 'Y';
                            
                        }
                        if (choice == 6)
                        {
                            block6 = 'X';
                            block3 = 'O';
                            validChoice = 'Y';
                            
                        }
                        if (choice == 7)
                        {
                            block7 = 'X';
                            block2 = 'O';
                            validChoice = 'Y';
                            
                        }
                        if (choice == 8)
                        {
                            block8 = 'X';
                            block1 = 'O';
                            validChoice = 'Y';
                            
                        }
                        if (choice == 9)
                        {
                            block9 = 'X';
                            validChoice = 'Y';
                            
                        }
                        while (validChoice == 'N')
                        {
                            if (choice > 9)
                            {
                                Console.WriteLine("Invalid choice.");
                                stringChoice = Console.ReadLine();
                                choice = Convert.ToInt32(stringChoice);
                                validChoice = 'N';
                            }
                            if (block1 == 'X' || block2 == 'X' || block3 == 'X' || block4 == 'X' || block5 == 'X' || block6 == 'X' || block7 == 'X' || block8 == 'X' || block9 == 'X')
                            {
                                Console.WriteLine("That block has already been chosen.");
                                stringChoice = Console.ReadLine();
                                choice = Convert.ToInt32(stringChoice);
                                validChoice = 'N';
                            }
                            if (block1 == 'Y' || block2 == 'Y' || block3 == 'Y' || block4 == 'Y' || block5 == 'Y' || block6 == 'Y' || block7 == 'Y' || block8 == 'Y' || block9 == 'Y')
                            {
                                Console.WriteLine("Thata block has already been chosen.");
                                stringChoice = Console.ReadLine();
                                choice = Convert.ToInt32(stringChoice);
                                validChoice = 'N';
                            }
                        }
                    }

                    validChoice = 'N';

                    winState = checkWin(block1, block2, block3, block4, block5, block6, block7, block8, block9);

                    if (winState == 'Y')
                    {
                        displayBoard(block1, block2, block3, block4, block5, block6, block7, block8, block9);
                        Console.WriteLine("Congrats! You have beaten the computer!");
                        playAgain = 'N';
                        break;
                    }
                    
                }

            }

        }

        static void displayBoard(char block1, char block2, char block3, char block4, char block5, char block6, char block7, char block8, char block9)
        {
            Console.WriteLine("*************************");
            Console.WriteLine("*       *       *       *");
            Console.WriteLine("*   {0}   *   {1}   *   {2}   *", block1, block2, block3);
            Console.WriteLine("*       *       *       *");
            Console.WriteLine("*************************");
            Console.WriteLine("*       *       *       *");
            Console.WriteLine("*   {0}   *   {1}   *   {2}   *", block4, block5, block6);
            Console.WriteLine("*       *       *       *");
            Console.WriteLine("*************************");
            Console.WriteLine("*       *       *       *");
            Console.WriteLine("*   {0}   *   {1}   *   {2}   *", block7, block8, block9);
            Console.WriteLine("*       *       *       *");
            Console.WriteLine("*************************");

        }

        static char checkWin(char block1, char block2, char block3, char block4, char block5, char block6, char block7, char block8, char block9)
        {
            char winState;

            if (block1 == 'X' && block2 == 'X' && block3 == 'X')
            {
                winState = 'Y';
                return winState;
            }
            if (block4 == 'X' && block5 == 'X' && block6 == 'X')
            {
                winState = 'Y';
                return winState;
            }
            if (block7 == 'X' && block8 == 'X' && block9 == 'X')
            {
                winState = 'Y';
                return winState;
            }
            if (block1 == 'X' && block4 == 'X' && block7 == 'X')
            {
                winState = 'Y';
                return winState;
            }
            if (block2 == 'X' && block5 == 'X' && block8 == 'X')
            {
                winState = 'Y';
                return winState;
            }
            if (block3 == 'X' && block6 == 'X' && block9 == 'X')
            {
                winState = 'Y';
                return winState;
            }
            if (block1 == 'X' && block5 == 'X' && block9 == 'X')
            {
                winState = 'Y';
                return winState;
            }
            else
            {
                winState = 'N';
                return winState;
            }
        }
    }
}

Advertisement
Congratulations on your first game!

One thing i noticed about your code is that you could use an array for all blocks because passing the blocks 1-9 separately isnt very efficient and nice looking codewise.

Perhaps you could even make the board a separate class and get those OOP vibes rollin´ :)

As said, you could use an array for the board like this:


const int EMPTY = 0;
const int BLOCK_X = 1;
const int BLOCK_O = 2;

int[][] board = new int[3][3];

and store the board in a separate class like "TTTBoard", containing functions for board operations, like

void TTTBoard.Clear();
int TTTBoard.HasWinner();
bool TTTBoard.SetBlock(int piece, int x, int y);
int TTTBoard.GetBlock(int x, int y);

This has a couple advantages:
- it saves a lot of code and it keeps all 'board-code' encapsulated in the board class

- it sets up a nice framework for experimenting with different AI techniques, like minimax (this might be an overkill for TTT, but it's a good way to get a feel for the algorithm).

Other points: you might want to experiment with the System.Drawing classes. It's realy easy to draw static images with .Net and capture mouse clicks within a grid. Also can start implementing a simple AI like this:

1 CheckForWinningMove()
2 CheckForAntiLosingMove()
3 CheckSettingUpDoubleWinLine()
4 CheckAntiLosingDoubleWinLine()
5 if (IsMiddleEmpty) SetBlockInMiddle() else SetBlockInACorner() else SetRandomBlock()

This should perform reasobly well for TTT. Hope this helps,

Edo
Edo
In cases where it is possible to do so, you should use a switch statement instead of many if statements.

 if (choice == 1)                        {                            block1 = 'X';                            block8 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 2)                        {                            block2 = 'X';                            block7 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 3)                        {                            block3 = 'X';                            block6 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 4)                        {                            block4 = 'X';                            block5 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 5)                        {                            block5 = 'X';                            block4 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 6)                        {                            block6 = 'X';                            block3 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 7)                        {                            block7 = 'X';                            block2 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 8)                        {                            block8 = 'X';                            block1 = 'O';                            validChoice = 'Y';                                                    }                        if (choice == 9)                        {                            block9 = 'X';                            validChoice = 'Y';                                                    }


could be better as

switch (choice){  case 1:    //also the common code should be factored into a routine    this.makeChoice(ref block1, ref block8, validChoice);  break;//  ... and so on}public void makeChoice(ref char block a, ref char block b, ref char validChoice){  a = 'X';  b = 'O';  validChoice = 'Y';}



Also, use (if possible)
if
else if
else if
else

instead of a bunch of if statements.

This topic is closed to new replies.

Advertisement