patternjavaMinor
Tic Tac Toe Android game
Viewed 0 times
androidtoetictacgame
Problem
Can you help me with suggestions regarding my code for Tic Tac Toe Game? This is my code, I've tested it and it works, but I feel like it can still be made shorter/clearer. I am not a good coder and it might look ugly to some.
```
public class MainActivity extends Activity {
/**
* Called when the activity is first created.
*/
@Override
public void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
setBoard();
}
int check[][];
int i,j;
Button b[][];
int player=0;
TextView textView;
Button newGame;
// Set up the game board.
private void setBoard()
{
b = new Button[4][4];
check = new int[4][4];
textView = (TextView) findViewById(R.id.textview1);
newGame = (Button) findViewById(R.id.newgame);
newGame.setOnClickListener (new View.OnClickListener(){
public void onClick(View v)
{
if(newGame.isEnabled())
{
textView.setText("Click button to start!");
player=0;
setBoard();
}
}
});
b[1][3] = (Button) findViewById(R.id.one);
b[1][2] = (Button) findViewById(R.id.two);
b[1][1] = (Button) findViewById(R.id.three);
b[2][3] = (Button) findViewById(R.id.four);
b[2][2] = (Button) findViewById(R.id.five);
b[2][1] = (Button) findViewById(R.id.six);
b[3][3] = (Button) findViewById(R.id.seven);
b[3][2] = (Button) findViewById(R.id.eight);
b[3][1] = (Button) findViewById(R.id.nine);
for (i = 1; i <= 3; i++) {
for (j = 1; j <= 3; j++)
check[i][j] = 2;
}
// add the click listeners for each button
for (i = 1; i <= 3; i++) {
for (j = 1; j <= 3; j++) {
b[i][j].setOnClickListener(new M
```
public class MainActivity extends Activity {
/**
* Called when the activity is first created.
*/
@Override
public void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
setBoard();
}
int check[][];
int i,j;
Button b[][];
int player=0;
TextView textView;
Button newGame;
// Set up the game board.
private void setBoard()
{
b = new Button[4][4];
check = new int[4][4];
textView = (TextView) findViewById(R.id.textview1);
newGame = (Button) findViewById(R.id.newgame);
newGame.setOnClickListener (new View.OnClickListener(){
public void onClick(View v)
{
if(newGame.isEnabled())
{
textView.setText("Click button to start!");
player=0;
setBoard();
}
}
});
b[1][3] = (Button) findViewById(R.id.one);
b[1][2] = (Button) findViewById(R.id.two);
b[1][1] = (Button) findViewById(R.id.three);
b[2][3] = (Button) findViewById(R.id.four);
b[2][2] = (Button) findViewById(R.id.five);
b[2][1] = (Button) findViewById(R.id.six);
b[3][3] = (Button) findViewById(R.id.seven);
b[3][2] = (Button) findViewById(R.id.eight);
b[3][1] = (Button) findViewById(R.id.nine);
for (i = 1; i <= 3; i++) {
for (j = 1; j <= 3; j++)
check[i][j] = 2;
}
// add the click listeners for each button
for (i = 1; i <= 3; i++) {
for (j = 1; j <= 3; j++) {
b[i][j].setOnClickListener(new M
Solution
This is the main problem with your code:
This one line is the biggest mistake in your code, and you use it everywhere, for instance:
There are a number of places you use that type of horrid duplicate structure. Get rid of it. A (more) acceptable alternative is to do something like this:
At a bare minimum, even something like this would still make your code more readable:
There is no need to check
Once you have done that, you should refactor out a number of methods from your code, but there is no point until you take care of the first issue.
if(player==0)This one line is the biggest mistake in your code, and you use it everywhere, for instance:
if (player == 0) {
b[x][y].setText("X");
check[x][y] = 0;
player = 1;
checkBoard();
} else {
b[x][y].setText("O");
check[x][y] = 1;
player = 0;
checkBoard();
}There are a number of places you use that type of horrid duplicate structure. Get rid of it. A (more) acceptable alternative is to do something like this:
b[x][y].setText(player.getSymbol());
check[x][y] = player;
player = player.next();
checkBoard();At a bare minimum, even something like this would still make your code more readable:
b[x][y].setText(symbol[player]);
check[x][y] = player;
player = (player + 1)%2;
checkBoard();There is no need to check
if(player==0) in your code, since you are doing the exact same thing either way - the only difference is that you are trivially modifying some of the parameters. Handle it by making a separate Player class to keep track of the differences those parameters.Once you have done that, you should refactor out a number of methods from your code, but there is no point until you take care of the first issue.
Code Snippets
if(player==0)if (player == 0) {
b[x][y].setText("X");
check[x][y] = 0;
player = 1;
checkBoard();
} else {
b[x][y].setText("O");
check[x][y] = 1;
player = 0;
checkBoard();
}b[x][y].setText(player.getSymbol());
check[x][y] = player;
player = player.next();
checkBoard();b[x][y].setText(symbol[player]);
check[x][y] = player;
player = (player + 1)%2;
checkBoard();Context
StackExchange Code Review Q#84137, answer score: 4
Revisions (0)
No revisions yet.