HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Game divide by 10

Submitted by: @import:stackexchange-codereview··
0
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;

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 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.