Text Based Minesweeper in Java

Question

I have been working on a text based minesweeper game in Java and I am looking for some ways to improve it. I worked through this project through JetBrains academy. I will attach the instructions of the final part of the project

Objectives In this final stage, your program should contain the following additional functionality:

Print the current state of the minefield starting with all unexplored cells at the beginning, ask the player for their next move with the message Set/unset mine marks or claim a cell as free:, treat the player's move according to the rules, and print the new minefield state. Ask for the player's next move until the player wins or steps on a mine. The player's input contains a pair of cell coordinates and a command: mine to mark or unmark a cell, free to explore a cell.

If the player explores a mine, print the field in its current state, with mines shown as X symbols. After that, output the message You stepped on a mine and failed!.

Generate mines like in the original game: the first cell explored with the free command cannot be a mine; it should always be empty. You can achieve this in many ways – it's up to you.

Use the following symbols to represent each cell's state:

. as unexplored cells

/ as explored free cells without mines around it

Numbers from 1 to 8 as explored free cells with 1 to 8 mines around them, respectively

X as mines

* as unexplored marked cells

Here is part of an example they gave

The greater-than symbol followed by a space (> ) represents the user input. Note that it's > not part of the input.

Example 1: the user loses after exploring a cell that contains a mine

 |123456789| -|---------| 1|.........| 2|.........| 3|.........| 4|.........| 5|.........| 6|.........| 7|.........| 8|.........| 9|.........| -|---------| Set/unset mines marks or claim a cell as free: > 3 2 free |123456789| -|---------| 1|.1///1...| 2|.1//12...| 3|11//1....| 4|////1....| 5|11111....| 6|.........| 7|.........| 8|.........| 9|.........| -|---------| Set/unset mines marks or claim a cell as free: > 1 1 free |123456789| -|---------| 1|11///1...| 2|.1//12...| 3|11//1....| 4|////1....| 5|11111....| 6|.........| 7|.........| 8|.........| 9|.........| -|---------| Set/unset mines marks or claim a cell as free: > 1 2 mine |123456789| -|---------| 1|11///1...| 2|*1//12...| 3|11//1....| 4|////1....| 5|11111....| 6|.........| 7|.........| 8|.........| 9|.........| -|---------| 

I will attach the code below. Any advice would be greatly appreciated but here are some of my overarching questions I have about my code

1. I wasn't sure how much code I should write in my main method. Does the game logic belong in the class or in the main method? One major debate I had with myself was whether to put the scanner in the main method or in the class itself (as you can see, I went with the latter). What would you have done?

2. I had to duplicate my code whenever I was doing operations with neigboring cells. For example, I use for (int i = Math.max(0, row - 1); i <= Math.min(8, row + 1) ; i++) and for (int j = Math.max(0, initialCol - 1); j <= Math.min(8, initialCol + 1); j++) whenever I need to iterate through a cells neighbors. Is there any way I could improve how I go about doing this?

3. Jetbrains academy introduced enums before completing this project but I didn't know how I would use them. Would they have made my code any better?

4. You may notice the lack of comments in my code. I don't use them too often (I know it's a bad habit to get into). Is the code self explanatory or does it need more comments?

And here is my code:

package minesweeper; import java.util.Objects; import java.util.Random; import java.util.Scanner; public class Main { public static void main(String[] args) { // write your code here Minefield game = new Minefield(); while (!game.hasLost || !game.hasWon) { game.nextTurn(); } } } class Minefield { int[][] board = new int[9][9]; boolean[][] mineLocations = new boolean[9][9]; boolean[][] marked = new boolean[9][9]; boolean[][] explored = new boolean[9][9]; boolean hasWon; boolean hasLost; boolean boardSet; static Scanner scanner = new Scanner(System.in); private int mines; public Minefield(){ System.out.print("How many mines do you want on the field? "); this.mines = scanner.nextInt(); this.showMinefield(); } public void nextTurn() { System.out.print("Set/unset mines marks or claim a cell as free: "); int col = scanner.nextInt() - 1; int row = scanner.nextInt() - 1; String command = scanner.next(); if (this.isExplored(row, col)) { System.out.println("This cell is already explored!"); } else { executeTurn(row, col, command); } } private void executeTurn(int row, int col, String command) { switch (command) { case "free": if (!boardSet) { boardSet = true; this.newBoard(row, col); explore(row, col); } if (this.hasMineAt(row, col)) { this.revealMinefield(); System.out.println("You stepped on a mine and failed!"); this.hasLost = true; } else { explore(row, col); } break; case "mine": this.marked[row][col] = !this.marked[row][col]; } this.checkWin(); this.showMinefield(); } private void newBoard(int initialRow, int initialCol){ //ensures the first move doesn't have any mines for (int i = Math.max(0, initialRow - 1); i <= Math.min(8, initialRow + 1); i++) { for (int j = Math.max(0, initialCol - 1); j <= Math.min(8, initialCol + 1); j++) { this.board[i][j] = -1; } } Random random = new Random(); for (int i = 0; i < this.mines; i++) { int mineRow , mineCol; do { mineRow = random.nextInt(9); mineCol = random.nextInt(9); } while (this.board[mineRow][mineCol] == -1); this.board[mineRow][mineCol] = -1; this.mineLocations[mineRow][mineCol] = true; } for (int i = Math.max(0, initialRow - 1); i <= Math.min(8, initialRow + 1); i++) { for (int j = Math.max(0, initialCol - 1); j <= Math.min(8, initialCol + 1); j++) { this.board[i][j] = 0; } } for (int i = 0; i < this.board.length; i++) { for (int j = 0; j < this.board[0].length; j++) { if (this.hasMineAt(i,j)) { for (int k = Math.max(0, i - 1); k <= Math.min(8, i + 1) ; k++) { for (int l = Math.max(0, j - 1); l <= Math.min(8, j + 1); l++) { if (!(k == i && l == j)) { this.board[k][l]++; } } } } } } if (board[initialRow][initialCol] != 0) { System.out.println("Error with mine spawning"); } } private void showMinefield() { System.out.println(" |123456789|"); System.out.println("-|---------|"); for (int i = 0; i < this.board.length; i++) { System.out.print((i + 1) + "|"); for (int j = 0; j < this.board[0].length; j++) { if (this.isMarked(i, j)) { System.out.print("*"); } else if (this.board[i][j] == 0 && this.isExplored(i, j)) { System.out.print("/"); } else if (this.isExplored(i, j)){ System.out.print(this.board[i][j]); } else { System.out.print("."); } } System.out.print("|"); System.out.println(); } System.out.println("-|---------|"); } private void revealMinefield() { System.out.println(" |123456789|"); System.out.println("-|---------|"); for (int i = 0; i < this.board.length; i++) { System.out.print((i + 1) + "|"); for (int j = 0; j < this.board[0].length; j++) { if (this.hasMineAt(i, j)) { System.out.print("X"); } else if (this.board[i][j] == 0 && this.isExplored(i, j)) { System.out.print("/"); } else if (this.isExplored(i, j)){ System.out.print(this.board[i][j]); } else { System.out.print("."); } } System.out.print("|"); System.out.println(); } System.out.println("-|---------|"); } private boolean hasMineAt(int row, int col) { return this.mineLocations[row][col]; } private boolean isMarked(int row, int col) { return this.marked[row][col]; } private boolean isExplored(int row, int col) { return this.explored[row][col]; } private void explore(int row, int col) { this.explored[row][col] = true; this.marked[row][col] = false; if (this.board[row][col] == 0) { for (int i = Math.max(0, row - 1); i <= Math.min(8, row + 1) ; i++) { for (int j = Math.max(0, col - 1); j <= Math.min(8, col + 1) ; j++) { if (!this.isExplored(i, j)) { this.explore(i, j); } } } } } private void checkWin(){ if (this.marked == this.mineLocations) { this.hasWon = true; return; } for (int i = 0; i < this.board.length; i++) { for (int j = 0; j < this.board[0].length; j++) { if (this.hasMineAt(i, j) && !this.isMarked(i,j)) { return; } } } this.hasWon = true; } } 

Answer 1

In my opinion, the main method should be used to set up and start the game. It should not be responsible for running it, so the loop would be in an incorrect place. The reason is that main method is just an entry point for a standalone applications. If your game should be a part of a larger application later, then you can no longer use the main method to run the game.

Setting up the scanner should be in the main method, but there are limitations to that. In a fully modular approach that follows single responsibility principle and separation of concerns, the game engine itself should not be dependant on a specific input method. You would have a separate IO-component that handles input from a scanner and converts it to game commands. I.e. it reads data from the scanner, parses them and passes them as method calls to the game engine and then outputs the game state in a way that is suitable to the player. When made this way, there could be different IO-components for console and GUI and they could both use the same game engine.

The IO-component can further be split into a view and a controller, but that may be something for later learning.

Enums could be used to represent the CellState of each cell in the 2d array: UNDISTURBED, REVEALED, FLAGGED. The data structures you have in the Minesweeper class don't lend themselves too well for this. Instead of having multiple arrays each containing a certain property of the locations in the field, you should have a single array containing objects that represent everything related to a single location. So create a class named Cell, put the boolean containsMine and CellState state in there and replace the four arrays with Cell[][] mineField = new Cell[9][9].

You have two fields called hasWon and hasLost. These should be replaced with a GameState enum that has the values ONGOING, WON, LOST. This way you safeguard yourself from accidentally endinf up in an invalid state where both hasWon and hasLost are true.

Answer 2

Does the game logic belong in the class or in the main method?

I'd argue that your question already has the answer in it, although through a minor hidden detail:

Does the game logic belong in the game class or in the main method?

One major debate I had with myself was whether to put the scanner in the main method or in the class itself (as you can see, I went with the latter). What would you have done?

This can be answered by looking at the concerns of the classes - i.e. what does the class do? A generally accepted principle to keep code understandable and extendable is the single responsibility principle, or more generally the separation of concerns.

Following these principles, IO and logic are usually separated into different layers and then connected through yet other classes. IO is already implemented in the standard libraries, so your task in this example could be to write a game logic class that looks something like this (pseudo code):

class Minefield { public Minefield(size) {...} public void mark(position, type) {...} public Board getBoard() {...} } 

and a connecting class similar to this:

class CommandLineMinesweeper { private void nextTurn {...} public void play() {...} } 

now the main method only has to call new CommandLineMinesweeper().play(), and the layers are separated nicely.

Answer 3

Users break stuff...

As soon as you start dealing with user input, no matter how you get it, they're going to end up sending you invalid input. You're not currently validating any user input, which makes it very easy to confuse the application. Consider what happens if the user types 'easy' when asked how many mines they want on the field (the application crashes because it's expecting an int). How about if the user answers 101 (the application hangs, because it after the field is full it can't find anywhere to place the remaining mines.

this this this

Generally in Java, you don't expect to see lots of this.xyz. Typically the only time you'll see this.xyz is to disambiguate names, for example between class members and parameters passed into methods.

executeTurn

A couple of things stand out about this method. Firstly, it mixes levels of abstraction/knowledge. Part of the method works at a high level, calling methods on the class to achieve goals (checkWin, showMinefield), whereas other parts work directly with fields of the class this.marked[row][col] = !this.marked[row][col];`. Having methods focus on a single level of abstraction can make it easier to reason about what the code is doing. It might seem like having single line functions is a bad idea, however if giving an activity a name makes it easier to reason about then it's usually worth doing.

The second thing that stands out is that when you make your first move, it explores the move twice. The first time when it creates the board and then immediately afterwards when it gets to the next if/else clause. Consider removing explore from this section of code (it's not needed).

if (!boardSet) { boardSet = true; this.newBoard(row, col); explore(row, col); } 

Reference checking

You're comparing two arrays by reference.

if (this.marked == this.mineLocations) {

This is only going to be true if both variables are referring to the same array, which they're not. You don't need this code, because it's already handled by the iteration check you've added afterwards.

You can only win and lose

This:

while (!game.hasLost || !game.hasWon) {

says keep playing if I haven't lost, or I haven't won. So, if I lose, it keeps going, because I haven't won yet. You have to trigger both win and lose conditions before the application exits. The logic you probably be:

while (!game.hasLost && !game.hasWon) {