patternjavaMinor
Game divide by 10
Viewed 0 times
dividegamestackoverflow
Problem
```
import javax.swing.JOptionPane;
public class DividedBy10v2{
public static void main( String[] args ){
int block[][] = new int[3][3];
int DetermineRow = 0; // The variable assigned for counting rows
int DetermineColumn = 0; // The variable assigned for counting columns
int blockRow,blockColumn;
int score = 0;
String inputRow, inputColumn;
//random a set of number
int randomSet[] = new int[100];
for(int i=0;i %d > %d \n\n", randomSet[round], randomSet[(round + 1)], randomSet[(round + 2)] );
//check over checking
if(block[0][0]>0&&block[0][1]>0&&block[0][2]>0){
if(block[1][2]>0&&block[1][1]>0&&block[1][2]>0){
if(block[2][0]>0&&block[2][1]>0&&block[2][2]>0){
System.out.println( "------ Game Over ------" );
//continue check
int ContinueCheck = JOptionPane.showConfirmDialog(null, "Do you want to continue ?", "Continue?", JOptionPane.YES_NO_OPTION);
if( ContinueCheck == JOptionPane.YES_OPTION){
//initialize the game board
for(DetermineRow=0;DetermineRow=block.length || blockRow=block.length || blockColumn=128){
block[0][2] = 0;
block[1][1] = 0;
block[2][0] = 0;
scoreCount -= 128;
}
if(scoreCount>=64){
block[0][0] = 0;
block[1][1] = 0;
block[2][2] = 0;
scoreCount -= 64;
}
if(scoreCount>=32){
block[0][2] = 0;
block[1][2] = 0;
block[2][2] = 0;
scoreCount -= 32;
}
if(scoreCount>=16){
block[0][1] = 0;
block[1][1] = 0;
block[2][1] = 0;
scoreCount -= 16;
import javax.swing.JOptionPane;
public class DividedBy10v2{
public static void main( String[] args ){
int block[][] = new int[3][3];
int DetermineRow = 0; // The variable assigned for counting rows
int DetermineColumn = 0; // The variable assigned for counting columns
int blockRow,blockColumn;
int score = 0;
String inputRow, inputColumn;
//random a set of number
int randomSet[] = new int[100];
for(int i=0;i %d > %d \n\n", randomSet[round], randomSet[(round + 1)], randomSet[(round + 2)] );
//check over checking
if(block[0][0]>0&&block[0][1]>0&&block[0][2]>0){
if(block[1][2]>0&&block[1][1]>0&&block[1][2]>0){
if(block[2][0]>0&&block[2][1]>0&&block[2][2]>0){
System.out.println( "------ Game Over ------" );
//continue check
int ContinueCheck = JOptionPane.showConfirmDialog(null, "Do you want to continue ?", "Continue?", JOptionPane.YES_NO_OPTION);
if( ContinueCheck == JOptionPane.YES_OPTION){
//initialize the game board
for(DetermineRow=0;DetermineRow=block.length || blockRow=block.length || blockColumn=128){
block[0][2] = 0;
block[1][1] = 0;
block[2][0] = 0;
scoreCount -= 128;
}
if(scoreCount>=64){
block[0][0] = 0;
block[1][1] = 0;
block[2][2] = 0;
scoreCount -= 64;
}
if(scoreCount>=32){
block[0][2] = 0;
block[1][2] = 0;
block[2][2] = 0;
scoreCount -= 32;
}
if(scoreCount>=16){
block[0][1] = 0;
block[1][1] = 0;
block[2][1] = 0;
scoreCount -= 16;
Solution
I think, when I run your program, that the combination of text-console and swing user input leads to a bad experience.
Really, given that you have the GUI input boxes, why do I have to type the row/column that you want entered? Why not present me with a grid of buttons, and I have the number I can add to the grid, and I just click where the number should go?
Doing such a grid would potentially even be easier than the text output you put on the console.
Despite that, I do appreciate that the code is runnable, and works.... until I press the cancel button, at which point the code throws a
As for the code style, it seems awfully complicated, with a lot of code repetition and 'magic numbers'. Changing the size of the grid would be very frustrating.... it is all hard-coded in.
So, you should set up a constant, say 'gridsize' and use that in many places:
Then you can fix a lot of places where you have magic numbers:
and so on.
The Show-Game-Board will become some form of loop:
Also, having variable names with capital-letters is not good java coding convention... The line:
is very off-putting to Java 'regulars'. For small loops like that, something simple like:
would be much more understandable. Note that I declared the row variable inside the for loop (
Now, inside that display loop, because you have complicated names, it has made a bug hard to see..... This is your code:
I will simply rename your variables.... and we can see a bug much more easily:
Can you see the bug now?
Should be:
Similarly, because you have complicated if-statements on the game-check, you have another bug there:
That should, on the second line be a 0 (I have marked the position with a '^'):
Using a loop (preferably in a method) would be better:
You can then, instead, call:
Note
Note how, at this point I have never referenced the actual size of the grid as being 3...? So all the code I have changed so far will work with a grid of any size....
Then,
Really, given that you have the GUI input boxes, why do I have to type the row/column that you want entered? Why not present me with a grid of buttons, and I have the number I can add to the grid, and I just click where the number should go?
Doing such a grid would potentially even be easier than the text output you put on the console.
Despite that, I do appreciate that the code is runnable, and works.... until I press the cancel button, at which point the code throws a
NumberFormatException.As for the code style, it seems awfully complicated, with a lot of code repetition and 'magic numbers'. Changing the size of the grid would be very frustrating.... it is all hard-coded in.
So, you should set up a constant, say 'gridsize' and use that in many places:
final int gridsize = 3;Then you can fix a lot of places where you have magic numbers:
int block[][] = new int[gridsize][gridsize];
....
randomSet[i] = (int) (Math.random() * gridsize * gridsize) + 1;and so on.
The Show-Game-Board will become some form of loop:
StringBuilder sb = new StringBuilder();
for (int i = 0; i < gridsize; i++) {
sb.append(String.format("%4d", i));
}Also, having variable names with capital-letters is not good java coding convention... The line:
for(DetermineRow=0;DetermineRow<block.length;DetermineRow++){ ....is very off-putting to Java 'regulars'. For small loops like that, something simple like:
for (int row = 0; row < gridsize; row++) { ...would be much more understandable. Note that I declared the row variable inside the for loop (
int row = 0). This is the normal way to define variables in for loops. It is not essential, but it actually leads to fewer bugs when the loop-variable is contained entirely inside the loop... (i.e. row is not visible outside the loop).Now, inside that display loop, because you have complicated names, it has made a bug hard to see..... This is your code:
for(DetermineRow=0;DetermineRow<block.length;DetermineRow++){
System.out.print( DetermineRow + " ");
for(DetermineColumn=0;DetermineColumn<block.length;DetermineColumn++){
if(block[DetermineRow][DetermineColumn]!=0){
System.out.print( block[DetermineRow][DetermineRow] + " " );
}else{
System.out.print( "_ " );
}
}
System.out.println();
}I will simply rename your variables.... and we can see a bug much more easily:
for(row=0;row<block.length;row++){
System.out.print( row + " ");
for(column=0;column<block.length;column++){
if(block[row][column]!=0){
System.out.print( block[row][row] + " " );
}else{
System.out.print( "_ " );
}
}
System.out.println();
}Can you see the bug now?
System.out.print( block[row][row] + " " );Should be:
System.out.print( block[row][column] + " " );Similarly, because you have complicated if-statements on the game-check, you have another bug there:
if(block[0][0]>0&&block[0][1]>0&&block[0][2]>0){
if(block[1][2]>0&&block[1][1]>0&&block[1][2]>0){
if(block[2][0]>0&&block[2][1]>0&&block[2][2]>0){That should, on the second line be a 0 (I have marked the position with a '^'):
if(block[0][0]>0&&block[0][1]>0&&block[0][2]>0){
if(block[1][2]>0&&block[1][1]>0&&block[1][2]>0){
^
if(block[2][0]>0&&block[2][1]>0&&block[2][2]>0){Using a loop (preferably in a method) would be better:
private static final boolean isComplete(int[][] grid) {
for (int row = 0; row < grid.length; row++) {
for (int col = 0; col < grid[row].length; col++) {
if (grid[row][col] == 0) {
return false;
}
}
}
return true; // all celss have values.
}You can then, instead, call:
//check over checking
if(isComplete(block)){
int ContinueCheck = JOptionPane.showConfirmDialog(null, "Do you want to continue ?", "Continue?", JOptionPane.YES_NO_OPTION);
if( ContinueCheck == JOptionPane.YES_OPTION){
...
continue;
}else if(ContinueCheck == JOptionPane.NO_OPTION) {
System.out.println( "-------------------------" );
System.out.println("Good Bye !");
break;
}
}Note
Note how, at this point I have never referenced the actual size of the grid as being 3...? So all the code I have changed so far will work with a grid of any size....
Then,
Code Snippets
final int gridsize = 3;int block[][] = new int[gridsize][gridsize];
....
randomSet[i] = (int) (Math.random() * gridsize * gridsize) + 1;StringBuilder sb = new StringBuilder();
for (int i = 0; i < gridsize; i++) {
sb.append(String.format("%4d", i));
}for(DetermineRow=0;DetermineRow<block.length;DetermineRow++){ ....for (int row = 0; row < gridsize; row++) { ...Context
StackExchange Code Review Q#36409, answer score: 6
Revisions (0)
No revisions yet.