# Conditionals validation for Tic Tac Toe

I have been working with Java for a little more than a year. I recently have built a Tic Tac Toe game as an assignment for my Java class. After my instructor graded it, he wrote a comment around my validation method logic. Even though I got 100%, he said that the logic in my validation method is too cumbersome. He stated that I should look into a for orwhile statement in order to clean out some code in my validation method.

Is there really a way to put all my conditional if statements in to a for or while loop? And if so, I would like to know what logic goes behind that. This program had a set of five arrays but in this validation method I worked only with the JButton array.

JButton [] button = new JButton [9];
public void validate()
{
if(button[0].getText().equals(button[1].getText()) && button[1].getText().equals(button[2].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[0].getText());
gameOver();
return;
}
else if(button[3].getText().equals(button[4].getText()) && button[4].getText().equals(button[5].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[3].getText());
gameOver();
return;
}
else if(button[6].getText().equals(button[7].getText()) && button[7].getText().equals(button[8].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[6].getText());
gameOver();
return;
}
else if(button[0].getText().equals(button[3].getText()) && button[3].getText().equals(button[6].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[0].getText());
gameOver();
return;
}
else if(button[1].getText().equals(button[4].getText()) && button[4].getText().equals(button[7].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[1].getText());
gameOver();
return;
}
else if(button[1].getText().equals(button[4].getText()) && button[4].getText().equals(button[7].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[1].getText());
gameOver();
return;
}
else if(button[2].getText().equals(button[5].getText()) && button[5].getText().equals(button[8].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[2].getText());
gameOver();
return;
}
else if(button[0].getText().equals(button[4].getText()) && button[4].getText().equals(button[8].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[0].getText());
gameOver();
return;
}
else if(button[2].getText().equals(button[4].getText()) && button[4].getText().equals(button[6].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[2].getText());
gameOver();
return;
}

int i;

for(i=0;i<button.length;i++)
{
if(button[i].isEnabled())
{
break;
}
}

if(i == button.length)
{
JOptionPane.showMessageDialog(null,"This was a Draw");
}
}

-

Start with replacing common code with function calls. For example, you can create a method for checking if an entire row has the same text:

bool checkRow(int row)
{
int col = row*3;
return button[col].getText().equals(button[col+1].getText())
&& button[col+1].getText().equals(button[col+2].getText());
}


Write similar methods for columns and diagonal. Then you can do something like that:

bool checkWin()
{
for(int i=0; i<3; i++)
{
if(checkRow(i))
return true;
if(checkCol(i))
return true;
}
if(checkMajorDiag())
return true;
if(checkMinorDiag())
return true;
return false;
}


And then your entire if-else chain will be replaced with:

if(checkWin())
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[2].getText());
gameOver();
return;
}


The code is now much more readable.

Two major points you'd want to remember:

1. Duplicate code leads to bugs

Let's say you'd have to change the message printed to the user - when the message appears several times this task is tedious, plus in a real (big and complex) situation there's a good chance you'll forget to change one of the usages. In the new version, there's only one line to change.

2. Atomic operations are always better

When I reviewed your code, I couldn't tell immediately what each if condition means.

checkRow(0) means "check the first row". checkRow(0) is an atomic operation (atomic in this context means a single operation - a single function call).

button[0].getText().equals(button[1].getText()) && button[1].getText().equals(button[2].getText()) is much less understandable. But the real danger here that if the line was button[0].getText().equals(button[1].getText()) && button[2].getText().equals(button[2].getText()), you probably couldn't tell that one index was changed. Which means that debugging atomic operations is much easier.

-
But you always show button[2].getText() as the winner! –  Florian F Sep 25 at 15:59

You have the button[1], button[4] and button[7] check twice in your code! This is a 'classic' mistake when copy/pasting blocks:

    else if(button[1].getText().equals(button[4].getText()) && button[4].getText().equals(button[7].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[1].getText());
gameOver();
return;
}
else if(button[1].getText().equals(button[4].getText()) && button[4].getText().equals(button[7].getText()))
{
JOptionPane.showMessageDialog(null,"Thank you the winner is" + button[1].getText());
gameOver();
return;
}


This is a case where a preset table of values will be helpful to test your conditions.

Consider a structure which identifies what three buttons represent a winning condition:

private static final int[][] TRIPLES = {
{0, 1, 2},
{3, 4, 5},
{6, 7, 8},

{0, 3, 6},
{1, 4, 7},
{2, 5, 8},

{0, 4, 8},
{2, 4, 6}
}


This array represents the buttons that you check for a winning condition (except {1, 4, 7] is only in here once ;-) )

Now, with the above structure, consider the following loop:

for (int[] triple : TRIPLES) {
int a = triple[0];
int b = triple[1];
int c = triple[2];
if(    button[a].getText().equals(button[b].getText())
&& button[b].getText().equals(button[c].getText())) {

JOptionPane.showMessageDialog(null,
"Thank you the winner is" + button[a].getText());
gameOver();
return;

}
}


This checks all your conditions, and there is very little code duplciation. If you want to, you can extract out the button-checks as suggested by palacsint, but I find the above is quite readable.

-

+1 to @opd and a few other notes:

1. You could extract out another method:

private boolean buttonTextEquals(int index1, int index2) {
return button[index1].getText().equals(button[index2].getText());
}


And use that in the checkRow():

private boolean checkRow(int row) {
int col = row * 3;
return buttonTextEquals(col, col + 1) && buttonTextEquals(col + 1, col + 2);
}

2. You could use a more descriptive name here than i:

int i;
`

What's the purpose of this variable? Use that as a name.

3. There were other Tic-tac-toe questions recently with great answer, you should check them.

-