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

A grid and a menu walked into a program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gridintoprogrammenuwalkedand

Problem

A program that creates a grid 10x10 and assigns a random number in each tile. It then asks you if you want to:

  • create a new grid



  • view the current one



  • count how many of each number there are in the grid



  • sum the rows



  • sum the columns



  • exit the program



```
import java.util.Arrays;
import java.util.Random;
import java.util.Scanner;

public class tenxten {
static int numberRows = 10;
static int numberColumns = 10;
static int [][] grid = new int [numberColumns][numberRows];

private static int randomInt(int from, int to) {
Random rand = new Random();
return rand.nextInt(to - from + 1) + from;
}

private static void amountOfSpecificNumbers() {
int[] numbers = new int[numberColumns * numberRows];
for (int i = 1; i 6 || choice < 1) {
System.out.println("Pleas enter number 1, 2, 3, 4, 5, or 6");
while (!scanner.hasNextInt()) {
System.out.println("That's not even a number");
System.out.println("Pleas enter number 1, 2, 3, 4, 5, or 6");
scanner.next();
}
choice = scanner.nextInt();
}
return choice;
}

public static void main(String[] args) {
newField();
while(true) {
System.out.println("What do you want to do?");
System.out.println("1. Get a new field");
System.out.println("2. Show current field");
System.out.println("3. Count the numbers in the current field");
System.out.println("4. Sum all rows");
System.out.println("5. Sum all columns");
System.out.println("6. Exit program");
Scanner scanner = new Scanner(System.in);

int choice = readInt(scanner);

if (choice == 1){
newField();
} else if (choice == 2){
showField();
} else if (choice == 3){
amountOfSpecificNumbers();

Solution

public class tenxten {


Java classes should start with a capital letter, and according to Java conventions should be named with something called "PascalCase". A name like TenXTen would adhere to that convention.

static int numberRows = 10;
static int numberColumns = 10;


These are effectively used as constants (they do not change). Therefore they can be:

private static final int NUMBER_ROWS = 10;
private static final int NUMBER_COLUMNS = 10;


(Constants are by convention named with ALL_CAPS_AND_UNDERLINES)

static int [][] grid = new int [numberColumns][numberRows];


At one place you write grid[y][x] and in others grid[x][y]

Luckily for you, it has the same dimensions so you won't notice, but should it be the following?

static int [][] grid = new int [numberRows][numberColumns];


Random rand = new Random();


You are currently creating one Random each time you are generating a number. Random objects are meant to be re-used (for "better randomization" - I know it sounds fuzzy but trust me on this one).

for (int y = 0; y < 10; y++) {
            for (int x = 0; x < 10; x++) {


and

for (int i = 1; i < 10; i++) {
        for (int y = 0; y < 10; y++) {
            for (int x = 0; x < 10; x++) {


Use the constants for the upper bound for x and y here.

newField has some duplication from showField. It might be better remove the output from newField and call the methods like this:

newField();
showField();


I believe your readInt method can be rewritten using do-while,

int choice;
do {
    System.out.println("Pleas enter number 1, 2, 3, 4, 5, or 6");
    while (!scanner.hasNextInt()) {
        System.out.println("That's not even a number");
        System.out.println("Pleas enter number 1, 2, 3, 4, 5, or 6");
        scanner.next();
    }
    choice = scanner.nextInt();
}
while (choice > 6 || choice < 1);
return choice;


Your amountOfSpecificNumbers() method can be simplified in a couple of ways:

  • use numbers[i]++; instead of numbers[i] += i; and you won't have to divide by i in the output.



  • don't use the outer loop, use a loop after the nested loop instead



-
int[] numbers doesn't need to be that big, it is currently 100 in size but only needs to be 10.

private static void amountOfSpecificNumbers() {
    int[] numbers = new int[10];
    for (int y = 0; y < NUMBER_ROWS; y++) {
        for (int x = 0; x < NUMBER_COLUMNS; x++) {
            int value = grid[y][x];
            numbers[value]++;
        }
    }
    for (int i = 0; i < numbers.length; i++) {
        System.out.println(" " + numbers[i] + " " + i + "s" );
    }
}


A little nitpick: Sometimes you are writing

int[] array


and sometimes

int array[]


while both works in Java, I would recommend sticking to one (I personally prefer int[] array)

Finally, imagine if you would have the requirement to handle more than one TenXTen grid at a time. Your program would really need TenXTen as an independent class in that case. (Currently, it doesn't need it, but it would be useful).

Many of your methods are returning void and doing the output inside the method. It is a better idea to return the values required for the output, and do the output outside the method itself.

Imagine a TenXTen grid ...who said it has to be 10 x 10 at all times? Consider the name NumberGrid... anyway, consider a class with these methods:

  • void generate()



  • int[] amountOfSpecificNumbers()



  • int[] sumOfColumns()



  • int[] sumOfRows()



  • void showField()



Then you would be able to use this methods for example like the following:

public static void main(String[] args) {
    NumberGrid grid = new NumberGrid(20, 10);
    grid.showField();
    System.out.println(Arrays.toString(grid.sumOfColumns()));
    grid.generate();
}


etc... you might want to read up on Java Classes and Objects for that.

Code Snippets

public class tenxten {
static int numberRows = 10;
static int numberColumns = 10;
private static final int NUMBER_ROWS = 10;
private static final int NUMBER_COLUMNS = 10;
static int [][] grid = new int [numberColumns][numberRows];
static int [][] grid = new int [numberRows][numberColumns];

Context

StackExchange Code Review Q#78014, answer score: 25

Revisions (0)

No revisions yet.