Well, here I have a tic-tac-toe app. I have made it properly, but it has a small issue. When it is a draw, it shows me the message like 8 times and I am not able to correct this alone. Everything else is alright. In the tutorial I have found it doesn't adress this specific issue, so what am I doing wrong?
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
}
string[] table = new string[9];
int countTurn = 0;
public String returnSymbol(int turn)
{
if (turn % 2 == 0)
{
return "O";
}
else
return "X";
}
public System.Drawing.Color determineColor(string symbol)
{
if(symbol.Equals("X"))
{
return System.Drawing.Color.LawnGreen;
}
else
{
return System.Drawing.Color.Aqua;
}
}
public void CheckDraw()
{
int count = 0;
for (int i = 0; i < table.Length; i )
{
if (table[i] != null)
count ;
if (count == 9)
{
MessageBox.Show("That's a draw!", "We have no winner :(", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
}
}
}
public void checkForWinner()
{
for(int i=0;i<8; i )
{
string combination = "";
int one=0, two=0, three = 0;
switch(i)
{
case 0:
combination = table[0] table[4] table[8];
one = 0;
two = 4;
three = 8;
break;
case 1:
combination = table[2] table[4] table[6];
one = 2;
two = 4;
three = 6;
break;
case 2:
combination = table[0] table[1] table[2];
one = 0;
two = 1;
three = 2;
break;
case 3:
combination = table[3] table[4] table[5];
one = 3;
two = 4;
three = 5;
break;
case 4:
combination = table[6] table[7] table[8];
one = 6;
two = 7;
three = 8;
break;
case 5:
combination = table[0] table[3] table[6];
one = 0;
two = 3;
three = 6;
break;
case 6:
combination = table[1] table[4] table[7];
one = 1;
two = 4;
three = 7;
break;
case 7:
combination = table[2] table[5] table[8];
one = 2;
two = 5;
three = 8;
break;
}
if(combination.Equals("OOO"))
{
changeColor(one);
changeColor(two);
changeColor(three);
MessageBox.Show("O has won the game!", "We have a winner!", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
}
else
if(combination.Equals("XXX"))
{
changeColor(one);
changeColor(two);
changeColor(three);
MessageBox.Show("X has won the game!", "We have a winner!", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
}
else
{
CheckDraw();
}
}
}
public void reset()
{
button1.Text = "";
button2.Text = "";
button3.Text = "";
button4.Text = "";
button5.Text = "";
button6.Text = "";
button7.Text = "";
button8.Text = "";
button9.Text = "";
button1.BackColor=System.Drawing.Color.White;
button2.BackColor=System.Drawing.Color.White;
button3.BackColor=System.Drawing.Color.White;
button4.BackColor=System.Drawing.Color.White;
button5.BackColor=System.Drawing.Color.White;
button6.BackColor=System.Drawing.Color.White;
button7.BackColor=System.Drawing.Color.White;
button8.BackColor=System.Drawing.Color.White;
button9.BackColor=System.Drawing.Color.White;
countTurn = 0;
table = new string[9];
}
public void changeColor(int number)
{
switch(number){
case 0:
button1.BackColor = System.Drawing.Color.OrangeRed;
break;
case 1:
button2.BackColor = System.Drawing.Color.OrangeRed;
break;
case 2:
button3.BackColor = System.Drawing.Color.OrangeRed;
break;
case 3:
button4.BackColor = System.Drawing.Color.OrangeRed;
break;
case 4:
button5.BackColor= System.Drawing.Color.OrangeRed;
break;
case 5:
button6.BackColor = System.Drawing.Color.OrangeRed;
break;
case 6:
button7.BackColor = System.Drawing.Color.OrangeRed;
break;
case 7:
button8.BackColor = System.Drawing.Color.OrangeRed;
break;
case 8:
button9.BackColor = System.Drawing.Color.OrangeRed;
break;
}
}
private void button1_Click(object sender, EventArgs e)
{
countTurn ;
table[0]=returnSymbol(countTurn);
button1.BackColor = determineColor(table[0]);
button1.Text = table[0];
checkForWinner();
}
private void button2_Click(object sender, EventArgs e)
{
countTurn ;
table[1] = returnSymbol(countTurn);
button2.BackColor = determineColor(table[1]);
button2.Text = table[1];
checkForWinner();
}
private void button3_Click(object sender, EventArgs e)
{
countTurn ;
table[2] = returnSymbol(countTurn);
button3.BackColor = determineColor(table[2]);
button3.Text = table[2];
checkForWinner();
}
private void button4_Click(object sender, EventArgs e)
{
countTurn ;
table[3] = returnSymbol(countTurn);
button4.BackColor = determineColor(table[3]);
button4.Text = table[3];
checkForWinner();
}
private void button5_Click(object sender, EventArgs e)
{
countTurn ;
table[4] = returnSymbol(countTurn);
button5.BackColor = determineColor(table[4]);
button5.Text = table[4];
checkForWinner();
}
private void button6_Click(object sender, EventArgs e)
{
countTurn ;
table[5] = returnSymbol(countTurn);
button6.BackColor = determineColor(table[5]);
button6.Text = table[5];
checkForWinner();
}
private void button7_Click(object sender, EventArgs e)
{
countTurn ;
table[6] = returnSymbol(countTurn);
button7.BackColor = determineColor(table[6]);
button7.Text = table[6];
checkForWinner();
}
private void button8_Click(object sender, EventArgs e)
{
countTurn ;
table[7] = returnSymbol(countTurn);
button8.BackColor = determineColor(table[7]);
button8.Text = table[7];
checkForWinner();
}
private void button9_Click(object sender, EventArgs e)
{
countTurn ;
table[8] = returnSymbol(countTurn);
button9.BackColor = determineColor(table[8]);
button9.Text = table[8];
checkForWinner();
}
private void TryAgainBtn_Click(object sender, EventArgs e)
{
reset();
}
}
}
CodePudding user response：
The problem is that your CheckDraw method is called within the outer for loop for(int i=0;i<8; i ). Change CheckDraw to return a bool like so:
public bool CheckDraw()
{
int count = 0;
for (int i = 0; i < table.Length; i )
{
if (table[i] != null)
count ;
if (count == 9)
{
MessageBox.Show("That's a draw!", "We have no winner :(", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
return true;
}
}
return false;
}
And then in CheckForWinner() change
else
{
CheckDraw();
}
to
else
{
if (CheckDraw())
{
break;
}
}
As a side note: I think your implementation allows to change the status of fields that have already been played. So you can win the game by changing an O to an X...
CodePudding user response：
You can actually simplify your CheckDraw function to the following
public void CheckDraw()
{
for (int i = 0; i < table.Length; i )
{
//on the first `null` entry it can't be a draw anymore, so return
if (table[i] == null)
return;
}
//if the loop found a value in all cells it's a draw ...
MessageBox.Show("That's a draw!", "We have no winner :(", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
}
or even simpler if you use LINQ
public void CheckDraw()
{
if (table.All(x => x != null)) {
MessageBox.Show("That's a draw!", "We have no winner :(", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
}
}
But that function is not your real problem. The problem is in checkForWinner. You should exit once you found a winner. And only if none of the winning situations leads to a winner, check also for a draw.
public void checkForWinner()
{
for(int i=0;i<8; i )
{
string combination = "";
int one=0, two=0, three = 0;
switch(i)
{
...
}
if(combination.Equals("OOO"))
{
...
MessageBox.Show("O has won the game!", "We have a winner!", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
return; //return from checkforWinner as we already have a winner
}
else if(combination.Equals("XXX"))
{
...
MessageBox.Show("X has won the game!", "We have a winner!", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
return; //return from checkforWinner as we already have a winner
}
}
//if we arrived here, it could still be a draw ...
checkDraw();
}
You could also dramatically simplify your checkForWinner if you create a List of winning situations beforehand and then just iterate over the situations and check it. Because the checking is always the same: Check if the three entries in the table are equal. If yes, the respective player has won the game ...
public void checkForWinner()
{
var winningList = new List<(int a, int b, int c)> {
(0, 1, 2),
(3, 4, 5),
//...
};
foreach (var t in winningList) {
if (table[t.a] != null && table[t.a] == table[t.b] && table[t.a] == table[t.c]) {
changeColor(t.a);
changeColor(t.b);
changeColor(t.c);
MessageBox.Show($"{table[t.a]} has won the game!", ...);
return; //return from checkforWinner as we already have a winner
}
}
//if we arrived here, it could still be a draw ...
CheckDraw();
}
And as said in another comment: Most of your button click handlers only differ in the index of the table lookup. You could easily attach a Tag to each button (or determine it even from the name of the button) so you could use the same clickhandler for all buttons instead of having to repeat the same code 9 times ...