4
\$\begingroup\$

This game works out the basic of a Tic Tac Toe game. I am looking to see if this can be redone in any way to make the code more smaller since this is really big.

private string player = "x";
private string cpu = "o";
public Form1()
{
    InitializeComponent();
    Buttons();
    Grid();

}

private void Form1_Load(object sender, EventArgs e)
{

}

Button[] buttons;
private bool turn = true;
public void Game(object sender, EventArgs e)
{
    Button clicked = (Button)sender;
    if (turn)
    {
        clicked.Text = player;
    }
    else {
        clicked.Text = cpu;
    }

    turn = !turn;
    clicked.Enabled = false;

    winners();

}


public void winnersreset()
{
    for (int i = 0; i < buttons.Length; i++) {
        buttons[i].Enabled = true;
        buttons[i].BackColor = Color.White;
        buttons[i].Text = "";
    }
}
public bool winners()
{
    bool winner = false;
    if (buttons[0].Text == player && buttons[1].Text == player && buttons[2].Text == player)
    {
        buttons[0].BackColor = Color.Blue;
        buttons[1].BackColor = Color.Blue;
        buttons[2].BackColor = Color.Blue;
        MessageBox.Show("X is Winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[3].Text == player && buttons[4].Text == player && buttons[5].Text == player)
    {
        buttons[3].BackColor = Color.Blue;
        buttons[4].BackColor = Color.Blue;
        buttons[5].BackColor = Color.Blue;
        MessageBox.Show("X is Winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[6].Text == player && buttons[7].Text == player && buttons[8].Text == player)
    {
        buttons[6].BackColor = Color.Blue;
        buttons[7].BackColor = Color.Blue;
        buttons[8].BackColor = Color.Blue;
        MessageBox.Show("X is Winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[0].Text == player && buttons[4].Text == player && buttons[8].Text == player)
    {
        buttons[0].BackColor = Color.Blue;
        buttons[4].BackColor = Color.Blue;
        buttons[8].BackColor = Color.Blue;
        MessageBox.Show("X is Winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[2].Text == player && buttons[4].Text == player && buttons[6].Text == player)
    {
        buttons[2].BackColor = Color.Blue;
        buttons[4].BackColor = Color.Blue;
        buttons[6].BackColor = Color.Blue;
        MessageBox.Show("X is Winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[0].Text == player && buttons[3].Text == player && buttons[6].Text == player)
    {
        buttons[0].BackColor = Color.Blue;
        buttons[3].BackColor = Color.Blue;
        buttons[6].BackColor = Color.Blue;
        MessageBox.Show("X is Winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[2].Text == player && buttons[5].Text == player && buttons[8].Text == player)
    {
        buttons[2].BackColor = Color.Blue;
        buttons[5].BackColor = Color.Blue;
        buttons[8].BackColor = Color.Blue;
        MessageBox.Show("X is Winner");
        winnersreset();
        winner = true;
    }

    //O winner//
    if (buttons[0].Text == cpu && buttons[1].Text == cpu && buttons[2].Text == cpu)
    {
        buttons[0].BackColor = Color.Red;
        buttons[1].BackColor = Color.Red;
        buttons[2].BackColor = Color.Red;
        MessageBox.Show("O is winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[3].Text == cpu && buttons[4].Text == cpu && buttons[5].Text == cpu)
    {
        buttons[3].BackColor = Color.Red;
        buttons[4].BackColor = Color.Red;
        buttons[5].BackColor = Color.Red;
        MessageBox.Show("O is winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[6].Text == cpu && buttons[7].Text == cpu && buttons[8].Text == cpu)
    {
        buttons[6].BackColor = Color.Red;
        buttons[7].BackColor = Color.Red;
        buttons[8].BackColor = Color.Red;
        MessageBox.Show("O is winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[0].Text == cpu && buttons[4].Text == cpu && buttons[8].Text == cpu)
    {
        buttons[0].BackColor = Color.Red;
        buttons[4].BackColor = Color.Red;
        buttons[8].BackColor = Color.Red;
        MessageBox.Show("O is winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[2].Text == cpu && buttons[4].Text == cpu && buttons[6].Text == cpu)
    {
        buttons[2].BackColor = Color.Red;
        buttons[4].BackColor = Color.Red;
        buttons[6].BackColor = Color.Red;
        MessageBox.Show("O is winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[0].Text == cpu && buttons[3].Text == cpu && buttons[6].Text == cpu)
    {
        buttons[0].BackColor = Color.Red;
        buttons[3].BackColor = Color.Red;
        buttons[6].BackColor = Color.Red;
        MessageBox.Show("O is winner");
        winnersreset();
        winner = true;
    }
    else if (buttons[2].Text == cpu && buttons[5].Text == cpu && buttons[8].Text == cpu)
    {
        buttons[2].BackColor = Color.Red;
        buttons[5].BackColor = Color.Red;
        buttons[8].BackColor = Color.Red;
        MessageBox.Show("O is winner");
        winnersreset();
        winner = true;
    }
    return winner;
}

public void Grid()
{
    this.Size = new System.Drawing.Size(300, 300);
    buttons[0].Parent = this;
    buttons[0].Location = new Point(0, 10);
    buttons[1].Parent = this;
    buttons[1].Location = new Point(100, 10);
    buttons[2].Parent = this;
    buttons[2].Location = new Point(200, 10);

    buttons[3].Parent = this;
    buttons[3].Location = new Point(0, 60);
    buttons[4].Parent = this;
    buttons[4].Location = new Point(100, 60);
    buttons[5].Parent = this;
    buttons[5].Location = new Point(200, 60);

    buttons[6].Parent = this;
    buttons[6].Location = new Point(0, 110);
    buttons[7].Parent = this;
    buttons[7].Location = new Point(100, 110);
    buttons[8].Parent = this;
    buttons[8].Location = new Point(200, 110);

    for (int i = 0; i < buttons.Length; i++)
    {
        buttons[i].Click += new EventHandler(Game);
        buttons[i].BackColor = Color.White;
    }
}

public void Buttons()
{
    buttons = new Button[9];
    buttons[0] = new Button();
    buttons[1] = new Button();
    buttons[2] = new Button();
    buttons[3] = new Button();
    buttons[4] = new Button();
    buttons[5] = new Button();
    buttons[6] = new Button();
    buttons[7] = new Button();
    buttons[8] = new Button();
}
\$\endgroup\$
5
\$\begingroup\$

So I suppose the answer to your question can go in a few different routes. Best practices would suggest that you write tests, encapsulate and use some different design patterns. However my thoughts when looking at your code is that this might be a little frustrating to you. But your question also points to something that I think a few people will miss. Making 10 small classes to solve the same problem (in terms of lines of code) will be the same as 1 large class. I'll offer this suggestion as a rebuttal and an answer. Learn how to encapsulate in clear logical manners. You'll end up with 10 (just a random number) small classes. The point being that the smaller the class the easier it is to understand what it does.

To answer your question as a superficial answer would be to make methods to remove the duplicate code. Your winners method has TONS of duplicate code and would be greatly simplified like so

public bool winners()
{
    bool winner = false;
    winner |= TestForWinner(player, 0,1,2, "X is Winner", Color.Blue);
    winner |= TestForWinner(player, 3,4,5, "X is Winner", Color.Blue);
    winner |= TestForWinner(player, 6,7,8, "X is Winner", Color.Blue);
    winner |= TestForWinner(player, 0,4,8, "X is Winner", Color.Blue);
    winner |= TestForWinner(player, 2,4,6, "X is Winner", Color.Blue);
    winner |= TestForWinner(player, 0,3,6, "X is Winner", Color.Blue);
    winner |= TestForWinner(player, 2,5,8, "X is Winner", Color.Blue);
    winner |= TestForWinner(cpu, 0,1,2, "0 is Winner", Color.Red);
    winner |= TestForWinner(cpu, 3,4,5, "0 is Winner", Color.Red);
    winner |= TestForWinner(cpu, 6,7,8, "0 is Winner", Color.Red);
    winner |= TestForWinner(cpu, 0,4,8, "0 is Winner", Color.Red);
    winner |= TestForWinner(cpu, 2,4,6, "0 is Winner", Color.Red);
    winner |= TestForWinner(cpu, 0,3,6, "0 is Winner", Color.Red);
    winner |= TestForWinner(cpu, 2,5,8, "0 is Winner", Color.Red);

    return winner;
}
private boolean TestForWinner(string test, int index1, int index2, int index3, string winnerText, Color winningColor)
{
    if(buttons[index1] == test && buttons[index2] == test && buttons[index3] == test)
    {
        buttons[index1].BackColor = winningColor;
        buttons[index2].BackColor = winningColor;
        buttons[index3].BackColor = winningColor;

        MessageBox.Show(winnerText);
        winnerrest();
        return true;
    }
    return false;
}

out of curiosity I double checked the work and you missed two scenarios. A win in column 2 and a draw. My solution looks like this

    private bool HasWinner()
    {
        foreach (var player in players)
        {
            if (CheckForWinner(player, 0, 1, 2)) return true;//win row 1
            else if (CheckForWinner(player, 3, 4, 5)) return true;// win row 2
            else if (CheckForWinner(player, 6, 7, 8)) return true;// win row 3
            else if (CheckForWinner(player, 0, 3, 6)) return true;// win column 1
            else if (CheckForWinner(player, 1, 4, 7)) return true;// win column 2 <--missed this one you did
            else if (CheckForWinner(player, 2, 5, 8)) return true;// win column 3
            else if (CheckForWinner(player, 0, 4, 8)) return true;// win \
            else if (CheckForWinner(player, 2, 4, 6)) return true;// win /
        }
        if (!buttons.Any(x => x.Enabled))
        {
            MessageBox.Show("Cats Game");
            return true;
        }
        return false;
    }

I made a Player class which has a string called Marker, and a Color called WinningColor I have an array of Player and switch back and forth which one is current. This allows me to loop through each player checking for a win, and if no moves are left showing the user it's a draw and returning true. Might be worth trying to replicate what I did.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.