Sign in to follow this  
Profusion

[.net] Critique needed on first C# game

Recommended Posts

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;
            }
        }
    }
}

Share this post


Link to post
Share on other sites
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´ :)

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this