# A grid and a menu walked into a program

Posted on

Problem

A program that creates a grid 10×10 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 < 10; i++) {
for (int y = 0; y < 10; y++) {
for (int x = 0; x < 10; x++) {
if (grid[y][x] == i) {
numbers[i] += i;
}
}
}
System.out.println(" " + numbers[i] / i + " " + i + "s" );
}
}

private static void sumOfColumns() {
int sumOfColumns[] = new int[numberColumns];
for (int x = 0; x < numberColumns; x++) {
for (int y = 0; y < numberRows; y++) {
sumOfColumns[y] += grid[x][y];
}
}
System.out.println(Arrays.toString(sumOfColumns));
}

private static void sumOfRows() {
int sumOfRows[] = new int[numberColumns];
for (int x = 0; x < numberColumns; x++) {
for (int y = 0; y < numberRows; y++) {
sumOfRows[x] += grid[x][y];
}
}
System.out.println(Arrays.toString(sumOfRows));
}

private static void newField() {
for (int x = 0; x < numberColumns; x++) {
for (int y = 0; y < numberRows; y++) {
int randomNumber = (randomInt(1, 10));
grid[x][y] = randomNumber;
if (randomNumber < 10) {
System.out.print(" " + randomNumber + " ");
} else {
System.out.print(randomNumber + " ");
}
}
System.out.println();
}
}

private static void showField() {
for (int x = 0; x < numberColumns; x++) {
for (int y = 0; y < numberRows; y++) {
if (grid[x][y] < 10) {
System.out.print(" " + grid[x][y] + " ");
} else {
System.out.print(grid[x][y] + " ");
}
}
System.out.println();
}
}

int choice = 0;
while(choice > 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);

if (choice == 1){
newField();
} else if (choice == 2){
showField();
} else if (choice == 3){
amountOfSpecificNumbers();
} else if (choice == 4){
sumOfRows();
} else if (choice == 5){
sumOfColumns();
}else {
return;
}
}
}
}
``````

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

### Don’t store data in static fields

The biggest issue in this program is that all the data is stored in `static` fields.
It would make sense to make everything non-static except the main method,
so that you can have multiple instances.

### No need for multiple `Random` instances

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

It’s inefficient and completely unnecessary to generate a new instance of `Random` every time you want to generate a new int.
It would be better to create a single `Random` instance and reuse that.

### Simplify `if-else` chain with a `switch`

``````if (choice == 1){
newField();
} else if (choice == 2){
showField();
} else if (choice == 3){
// ...
``````

It’s better to use a `switch`:

``````switch (choice) {
case 1:
newField();
break;
case 2:
showField();
break;
// ...
}
``````

### Use consistent dimension constants

The `grid` is initialized using `numberColumns` and `numberRows` as the dimensions:

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

But then, everywhere else in the code,
you iterate using the hardcoded number 10 as the index bound
It would be better to reuse the `numberColumns` and `numberRows` in those iterations.
And, of course, since these are constants,
the common convention is name them with ALL CAP letters.

### Avoid code duplication

This `readInt` method has a duplicate string in it,
and it wastes an evaluation of `choice` the first time.

``````private static int readInt(Scanner scanner){
int choice = 0;
while(choice > 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;
}
``````

It would be better to eliminate the duplication and to convert to a do-while loop:

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

You can do even better and simplify without a loop, using an appropriate pattern in `scanner.hasNext`:

``````private static int readInt(Scanner scanner) {
String prompt = "Please enter number 1, 2, 3, 4, 5, or 6";
System.out.println(prompt);
while (!scanner.hasNext("[1-6]")) {
System.out.println(prompt);
scanner.next();
}
return scanner.nextInt();
}
``````