Simplifying Tic Tac Toe game

Question

I'm in a little bit of a pickle on my Tic Tac Toe program because I can't see how to simplify the code without breaking it. Do I need to make my code modular?

public partial class dglassAssign4 : Form
    {   
        string[] asImArr;
        const int ciArrSize = 9;
        int[] aiOrdArr;

        int turn = 1;
        bool click0 = false, click1 = false, click2 = false, click3 = false, click4 = false, click5 = false, click6 = false, click7 = false, click8 = false;
        bool x0, o0, x1, o1, x2, o2, x3, o3, x4, o4, x5, o5, x6, o6, x7, o7, x8, o8,over;

        public dglassAssign4()
        {
            asImArr = new string[] {"WO.jpg", "WX.jpg", "BO.jpg", "RX.jpg" };
            aiOrdArr = new int[ciArrSize];
            InitializeComponent();
        }
        private void btnGame_Click(object sender, EventArgs e)
        {            
            gameOver();
        }

        private void dglassAssign4_Load(object sender, EventArgs e)
        {                     
        }
        private void pb0_Click(object sender, EventArgs e)
        {
            if (click0==false)
            {                
                if(turn%2!=0)
                {
                    pb0.Image = Image.FromFile("WX.jpg");
                    x0 = true;
                }
                else
                {
                    pb0.Image = Image.FromFile("WO.jpg");
                    o0 = true; ;
                }
                turn++;
                click0 = true;
                endGame(turn);
            }          
        }
        private void pb1_Click(object sender, EventArgs e)
        {
            if (click1 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb1.Image = Image.FromFile("WX.jpg");
                    x1 = true;
                }
                else
                {
                    pb1.Image = Image.FromFile("WO.jpg");
                    o1 = true;
                }
                turn++;
                click1 = true;
                endGame(turn); 
            }           
        }
        private void pb2_Click(object sender, EventArgs e)
        {
            if (click2 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb2.Image = Image.FromFile("WX.jpg");
                    x2 = true;
                }
                else
                {
                    pb2.Image = Image.FromFile("WO.jpg");
                    o2 = true;
                }
                turn++;
                click2 = true;
                endGame(turn);
            }           
        }
        private void pb3_Click(object sender, EventArgs e)
        {
            if (click3 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb3.Image = Image.FromFile("WX.jpg");
                    x3 = true;
                }
                else
                {
                    pb3.Image = Image.FromFile("WO.jpg");
                    o3 = true;
                }
                turn++;
                click3 = true;
                endGame(turn);
            }           
        }
        private void pb4_Click(object sender, EventArgs e)
        {
            if (click4 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb4.Image = Image.FromFile("WX.jpg");
                    x4 = true;
                }
                else
                {
                    pb4.Image = Image.FromFile("WO.jpg");
                    o4 = true;
                }
                turn++;
                click4 = true;
                endGame(turn);
            }           
        }
        private void pb5_Click(object sender, EventArgs e)
        {
            if (click5 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb5.Image = Image.FromFile("WX.jpg");
                    x5 = true;
                }
                else
                {
                    pb5.Image = Image.FromFile("WO.jpg");
                    o5 = true;
                }
                turn++;
                click5 = true;
                endGame(turn);
            }           
        }
        private void pb6_Click(object sender, EventArgs e)
        {
            if (click6 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb6.Image = Image.FromFile("WX.jpg");
                    x6 = true;
                }
                else
                {
                    pb6.Image = Image.FromFile("WO.jpg");
                    o6 = true;
                }
                turn++;
                click6 = true;
                endGame(turn);
            }           
        }
        private void pb7_Click(object sender, EventArgs e)
        {
            if (click7 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb7.Image = Image.FromFile("WX.jpg");
                    x7 = true;
                }
                else
                {
                    pb7.Image = Image.FromFile("WO.jpg");
                    o7 = true;
                }
                turn++;
                click7 = true;
                endGame(turn);
            }           
        }        
        private void pb8_Click_1(object sender, EventArgs e)
        {
            if (click8 == false)
            {
                txtOut.Text = "";
                if (turn % 2 != 0)
                {
                    pb8.Image = Image.FromFile("WX.jpg");
                    x8 = true;
                }
                else
                {
                    pb8.Image = Image.FromFile("WO.jpg");
                    o8 = true;
                }
                turn++;
                click8 = true;
                endGame(turn);
            }           
        }    
        private void endGame(int turn)
        {
            if(turn >= 5)
            {
                if (x0 == true && x1 == true && x2 == true || x3 == true && x4 == true && x5 == true || x6 == true && x7 == true && x8 == true || x0==true && x3==true && x6 ==true || x1==true && x4==true && x7==true ||x2==true && x5==true && x8==true|| x0==true && x4==true && x8==true ||
                    x2 == true && x4 == true && x6 == true || o0 == true && o1 == true && o2 == true || o3 == true && o4 == true && o5 == true || o6 == true && o7 == true && o8 == true || o0 == true && o3 == true && o6 == true || o1 == true && o4 == true && o7 == true || o2 == true && o5 == true && o8 == true || o0 == true && o4 == true && o8 == true ||
                    o2 == true && o4 == true && o6 == true)
                {                    
                   txtOut.Text = ("Winner!!!");
                    over = true;
                    gameOver();
                }
            }



        }
        private void gameOver()
        {
            pb0.Image = null;
            pb1.Image = null;
            pb2.Image = null;
            pb3.Image = null;
            pb4.Image = null;
            pb5.Image = null;
            pb6.Image = null;
            pb7.Image = null;
            pb8.Image = null;
            click0 = false;
            click1 = false;
            click2 = false;
            click3 = false;
            click4 = false;
            click5 = false;
            click6 = false;
            click7 = false;
            click8 = false;
            x0 = false;
            x1 = false;
            x2 = false;
            x3 = false;
            x4 = false;
            x5 = false;
            x6 = false;
            x7 = false;
            x8 = false;
            o0 = false;
            o1 = false;
            o2 = false;
            o3 = false;
            o4 = false;
            o5 = false;
            o6 = false;
            o7 = false;
            o8 = false;
            turn = 1;

        }
   }
}

Answer 1

All your click event handlers can be condensed to one handler. Also instead of keeping track of which picturebox has been clicked you could simple disable the picturebox once it's been clicked. Possible something like this:

private void pb_Click_1(object sender, EventArgs e)
{
    PictureBox pb = (PictureBox)sender;
        txtOut.Text = "";
        if (turn % 2 != 0)
        {
            pb.Image = Image.FromFile("WX.jpg");
        }
        else
        {
            pb.Image = Image.FromFile("WO.jpg");
        }
        turn++;
        pb.Enabled = false;
        endGame(turn);
    }           
}

Assigning more than one control to a handler is easily done in VS via the event properties. Simply give the click event for each control the same method to use.

Answer 2

You should decouple (separate) your view (your form) from the logic. In this way you can easily change the presentation of game if e.g you decide to use WPF instead of Windows.Forms.

That beeing said, let us take a look at the "problems" in your code.

The first thing which I noticed is that you use arrays, but only for a part of your code like

int[] aiOrdArr;  

but for other important parts you are using single variables like

bool click0 = false, click1 = false, click2 = false, click3 = false, click4 = false, click5 = false, click6 = false, click7 = false, click8 = false;
bool x0, o0, x1, o1, x2, o2, x3, o3, x4, o4, x5, o5, x6, o6, x7, o7, x8, o8,over;  

You should define a class Cell which represents a cell of the board. This cell should have a state defining which of the players has marked it or if it is empty. This can be accomplished by using a Enum.

Then you should create a class Board containing the cells. Providing methods for "marking" a cell.

---

Your current code could still need some facelifting.

Instead of loading the images from file for every move, you should load each image once and then just assign this image to the Image property of the desired PictureBox.

---

You have named your class and variables poorly and if you come back in a couple of months looking at your code, you won't know what e.g aiOrdArr stands for.

Naming variables, methods and classes is sometimes hard if you want to do it right.

Some improvements

public partial class dglassAssign4
string[] asImArr;  

should be

 public partial class TicTacToeView
 string[] imageFileNames;  

Also based on the naming guidelines classes should be named using PascalCase casing.

---

The int[] aiOrdArr; isn't used so you can just remove it.

---

Instead of calling the method gameOver() (which should use PascalCase casing for the naming too) if the game has a winner, I would suggest to firstly rename this method to StartGame() and call it on a new created Button to start the game.

In its current state the game can't be restarted if no winner has been found.

Inside of the StartGame you should also set the txtOut.Text = ""; so you can remove this in the click handlers.

---

private void endGame(int turn)
{
    if(turn >= 5)
    {
        if (x0 == true && x1 == true && x2 == true || x3 == true && x4 == true && x5 == true || x6 == true && x7 == true && x8 == true || x0==true && x3==true && x6 ==true || x1==true && x4==true && x7==true ||x2==true && x5==true && x8==true|| x0==true && x4==true && x8==true ||
            x2 == true && x4 == true && x6 == true || o0 == true && o1 == true && o2 == true || o3 == true && o4 == true && o5 == true || o6 == true && o7 == true && o8 == true || o0 == true && o3 == true && o6 == true || o1 == true && o4 == true && o7 == true || o2 == true && o5 == true && o8 == true || o0 == true && o4 == true && o8 == true ||
            o2 == true && o4 == true && o6 == true)
        {                    
           txtOut.Text = ("Winner!!!");
            over = true;
            gameOver();
        }  
    }

}

By adding a guard clause you can save horizontal space and therefor improve the readability of the code.

Additional new lines, like in the end, don't add any value and should be removed.

Instead of checking if (x0 == true && x1 == true && x2 == true... you can just do if (x0 && x1 && x2 ....

Because you are initializing turn with 1 the check will be executed one time to often. After the first player marked his third cell turn will be 6.

This value should also be extracted to a meaningful constant to remove themagic number.

Implementing these points will lead to

private const int minimumTurns = 5;
private const int maximumTurns = 10;
private void endGame(int turn)
{
    if(turn < minimumTurns) { return; }

        if (x0 && x1 && x2 || x3 && x4 && x5 || x6 && x7 && x8 || 
            x0 && x3 && x6 || x1 && x4 && x7 || x2 && x5 && x8 ||  
            x0 && x4 && x8 || x2 && x4 && x6 || o0 && o1 && o2 ||  
            o3 && o4 && o5 || o6 && o7 && o8 || o0 && o3 && o6 ||  
            o1 && o4 && o7 || o2 && o5 && o8 || o0 && o4 && o8 ||  
            o2 && o4 && o6)
        {                    
            txtOut.Text = ("Winner!!!");
            over = true;
        }
        else if (turn == maximumTurns)
        { 
            txtOut.Text = ("No winner!!!");
            over = true;
        }
    }
}

Answer 3

You need to split this code up into methods.

private void PieceClick(ref bool click, ref bool xVal, ref bool oVal, Image img)
{
    if (!click)
    {                
        if (turn % 2 != 0)
        {
            img.Image = Image.FromFile("WX.jpg");
            xVal = true;
        }
        else
        {
            img.Image = Image.FromFile("WO.jpg");
            oVal = true;
        }
        turn++;

        click = true;
        endGame(turn);
    }
}

This can then be called as a method from within your click methods like this:

private void pb0_Click(object sender, EventArgs e)
{
    PieceClick(ref click0, ref x0, ref o0, (Image)sender, );
}

This may not run as is. Notice in the first place that I converted sender to an Image. This is sheer conjecture on my part, and will need to be changed to the type of the control you are clicking. sender tell the method which control is being clicked, so you don't need to have all this duplication in a single method.

If you have trouble getting the value of sender to change, you might need to use the ref keyword, but because this is a reference type, you probably don't need to as they are passed by reference automatically.

Note also that I changed the style if (click == false) to if (!click). This works the same, but is the style you typically use when working with boolean values.

You should put spaces around your operators, and name your variables a little more descriptively than pb0 or x0.

If I could run and test this entire program, I might be able to get this down into a single method that could replace all the pb#_Click functions, but this is all I see at the moment.