1
\$\begingroup\$

I made a Minesweeper game in Java 8, but I am not sure if the code is any good. Comments and questions are in the code.

Here's the code on Github (not commented though).

Game.java:

Main Class of the game, handles input and output and a number of other things (I am not sure whether I should remove this class or not).

import java.util.*;

public class Game {

Board board;

public Game(int length, int width,int numOfMines) {

    board = new Board(length, width, numOfMines);

    help();

    // First choice isn't guaranteed to be safe
    // TODO: generate the board when the user makes his first choice ( to make sure it's safe )

    System.out.println("Generating Board");

    start();

}

public void start() {
    while (true) {
        userInput();
    }
}

public static void main(String[] args) {

    // currently doesn't check if  numOfMines > (length*width)

    Scanner scan = new Scanner(System.in);
    System.out.print("Length: ");
    int length = scan.nextInt();

    System.out.print("Width: ");
   int width =  scan.nextInt();

    System.out.print("Number Of Mines: ");
    int numOfMines = scan.nextInt();

    new Game(length,width,numOfMines);
}


public void help() {
    System.out.println();
    System.out.println("Commands:");
    System.out.println("           \"help\" opens the help menu");
    System.out.println("           \"choose\" specify which tile you want to check");
    System.out.println("           \"flag\" specify which tile you want to flag");
    System.out.println("           \"restart\" start a new game");
    System.out.println("           \"quit\" to quit the game");
    System.out.println();
}

// handles user input (not sure if this is the best way to handle it)
// there is a lot of exceptions that can be caught here but I'm leaving them for now

void userInput() {
    Scanner scan = new Scanner(System.in);

    System.out.print("$ ");
    String userInput;

    userInput = scan.nextLine();
    userInput = userInput.trim().toLowerCase();

    int row;
    int column;

    switch (userInput)
    {
        case "help":

            help();
            break;

        case "choose":

            System.out.print("Row: ");
            row = scan.nextInt() - 1;

            System.out.print("Column: ");
            column = scan.nextInt() - 1;

            choose(row, column);
            board.printBoard();
            break;

        case "restart":

            // currently doesn't check if  numOfMines > (length*width)
            System.out.print("Length: ");
            int length = scan.nextInt();

            System.out.print("Width: ");
            int width =  scan.nextInt();

            System.out.print("Number Of Mines: ");
            int numOfMines = scan.nextInt();

            restart(length, width, numOfMines);
            break;

        case "flag":

            System.out.print("Row: ");
            row = scan.nextInt()-1;

            System.out.print("Column: ");
            column =  scan.nextInt()-1;

            flag(row, column);
            board.printBoard();
            break;

        case "quit":

            scan.close();
            System.exit(0);

        case "":
            break;
        default:
            System.out.println("Invalid input");
            break;
    }
}

// checks Input:
// if the cell is  a mine then the user losses( not implemented yet)
// if it has a value( value>0 ) then it reveals the cell then prints the board
// if the cell doesn't have a value it (value == 0)  then it calls reveal()

void choose(int row, int column) {
    Cell cell = board.getBoard()[row][column];
    if (cell.isMine()) {
        board.printBoard();
        System.out.println("Lose");
    } else if (cell.hasValue()) {
        cell.show();
        board.printBoard();
    } else if (!cell.hasValue()) {
        reveal(cell, new ArrayList<>(), new ArrayList<>(),0);
    }
}

// queue contains empty cells that hasn't been processed ( processing means checking its surroundingCells )
// adds the Cells to the queue(DOESN'T ADD THEM IF THEY'RE IN THE "PROCESSED" ARRAYLIST) (and reveals it) if their value is equal to "0"(Empty)
// if their value > 0 (and not a mine) then it just reveals them
// if the cell is a mine it doesn't reveal them
// should returns if queue.isEmpty
// TODO: Bug: returns in a weird way.

public void reveal(Cell cell, ArrayList<Cell> queue, ArrayList<Cell> processed,Integer i) {

    // debugging( number of recursions )
    i++;

    cell.show();

    if (queue.isEmpty()) {
        ArrayList<Cell> surroudingCells = cell.getSurroundingCells();
        for (Cell cell2 : surroudingCells) {
            if (!cell2.hasValue() && !cell.isMine()) {
                queue.add(cell2);
            } else if (cell2.hasValue()) {
                cell2.show();
            }
        }

        if (queue.isEmpty()) {

        } else {
            reveal(queue.get(0), queue, processed, i);
        }
    } else {
        for (Cell cell2 : cell.getSurroundingCells()) {
            if (queue.contains(cell2)) {

            } else if (processed.contains(cell2)) {

            } else if (!cell2.hasValue() && !cell2.isMine()) {
                queue.add(cell2);
            } else if (cell2.hasValue()) {
                cell2.show();
            } else if (cell2.isShown()) {

            } else if (cell2.isMine()) {

            }
        }

        processed.add(cell);
        queue.remove(cell);

        if (queue.isEmpty()) {
            return;
        } else {
            reveal(queue.get(0), queue, processed,i);
        }
    }
    // Debugging
    board.printBoard();
    System.out.println(i);
}

// sets isFlagged to true/false depending on its state

void flag(int row, int column) {

    Cell cell = board.getBoard()[row][column];
    cell.setFlagged(!cell.isFlagged());

}


// creates a new board

void restart(int length, int width, int numOfMines) {
    System.out.println("Generating new Board");
    board = new Board(length, width, numOfMines);
}
}

Board.java: Represents the board.

Creates an array filled with Cells, handles the properties of each cell, used by Game.java to communicate with cells.

import java.util.Random;
import java.util.Arrays;

public class Board {

private final int width;
private final int length;
private final int numOfMines;

private Cell[][] board;


public Board(int length, int width, int numOfMines) {

    this.width = width;
    this.length = length;
    this.numOfMines = numOfMines;

    board = new Cell[length][width];
    generate();
}

private void generate() {
    generateMines();
    generateNumbers();
    printSolvedBoard();
}

// creates a new cell that has a chance of being a mine (0.00001) ( The chance is low because I'm afraid that only the first few Cells are going to become mines)
// doesn't stop until there are the amount of mines specified by Game.java ( user )
// This is inefficient(loops the array 20000-30000 times when the number of mines = 20), I'm sure there is a better way to do this.


private void generateMines() {

    int currentNumOfMines = 0;
    Random random = new Random();


    while (currentNumOfMines < numOfMines) {

        for (int i = 0; i < length; i++) {
            for (int j = 0; j < width; j++) {

                double probability = random.nextDouble();

                if (board[i][j] == null) {
                    board[i][j] = new Cell(i, j, false, board.clone());
                } else if (board[i][j].isMine()) {

                } else if (probability > 0.99999 && currentNumOfMines < numOfMines) {
                    board[i][j] = new Cell(i, j, true, board.clone());
                    currentNumOfMines++;
                }
            }
        }
    }
}

// Calls Cell.setValue() method on every Cell in the board

private void generateNumbers() {
    for (int i = 0; i < length; i++) {
        for (int j = 0; j < width; j++) {
            if (board[i][j].isMine()) {

            } else {
                board[i][j].setValue();
            }
        }
    }
}
public void printBoard() {

    for (int i = 0; i < length; i++) {
        for (int j = 0; j < width; j++) {
            if (board[i][j].isMine()) {

            }
            System.out.print(" " + board[i][j].getSymbol());
        }
        System.out.println();
    }
}

public void printSolvedBoard() {

    for (int i = 0; i < length; i++) {
        for (int j = 0; j < width; j++) {
            if (board[i][j].isMine()) {

            }
            System.out.print(" " + board[i][j].getValue());
        }
        System.out.println();
    }
}

// I am not sure if I am overriding these correctly (hashCode(),equals())

@Override
public int hashCode() {
    return width * length * numOfMines + 13;
}

@Override
public boolean equals(Object obj) {
    Board boardObj = (Board) obj;
    return Arrays.deepEquals(boardObj.getBoard(), board);
}

public int getWidth() {
    return width;
}

public int getLength() {
    return length;
}

public int getNumOfMines() {
    return numOfMines;
}

public Cell[][] getBoard() {
    return board;
}
}

Cell.java:

Represents the Cell. Knows its position in the array, whether it is a mine or not, whether it is flagged/shown or not and it also has a clone of the array it is in (I'm not sure if that is a good idea).

import java.util.ArrayList;
import java.util.Arrays;

public class Cell {

// the Cells Position

private final int xPos;
private final int yPos;

// the Cells Properties
private final boolean isMine;
private boolean isFlagged;
private boolean isShown;

private String symbol;
private String value;

// the Cell's surroundings

private Cell[][] myBoard;
private ArrayList<Cell> surrondingCells;

public Cell(int x, int y, boolean isMine, Cell[][] board) {

    this.isMine = isMine;
    this.isFlagged = false;
    this.isShown = false;

    this.myBoard = board;
    surrondingCells = new ArrayList<>();

    this.xPos = x;
    this.yPos = y;
    symbol = "*";

    if (this.isMine) {
        value = "#";
    }
}

public boolean isMine() {
    return isMine;
}

public int getxPos() {
    return xPos;
}

public int getyPos() {
    return yPos;
}

public String getSymbol() {
    return symbol;
}

public void show() {
    symbol = value;
    isShown = true;
}

public boolean isShown() {
    return isShown;
}

public boolean hasValue() {

    if (isMine == true) return false;

    return Integer.parseInt(value) > 0;
}

public ArrayList<Cell> getSurroundingCells() {

    if (surrondingCells.isEmpty()) {
        setSurroundingCells();
    }

    return surrondingCells;
}

private void setSurroundingCells() {
    for (int i = xPos - 1; i <= xPos + 1; i++) {
        for (int j = yPos - 1; j <= yPos + 1; j++) {

            if (i == xPos && j == yPos) {

            } else {
                try {
                    surrondingCells.add(myBoard[i][j]);
                } catch (ArrayIndexOutOfBoundsException e) {
                    continue;
                }
            }
        }
    }
}

public String getValue() {
    return value;
}


// gets the surroundingCells from getSurroundingCells() then checks the number of mines in the ArrayList

public void setValue() {

    if (isMine) {
        return;
    }

    // if the board contains null then the method will exist (to avoid errors)

    if (Arrays.asList(myBoard).contains(null)) {
        return;
    }

    int surroundingMines = 0;

    for (Cell cell : getSurroundingCells()) {
        if (cell.isMine()) {
            surroundingMines++;
        }
    }

    value = Integer.toString(surroundingMines);
}

public boolean isFlagged() {
    return isFlagged;
}

public void setFlagged(boolean flagged) {
    isFlagged = flagged;

    if (isShown) {
        return;
    } else if (isFlagged) {
        symbol = "F";
    } else {
        if (isMine) {
            symbol = "#";
        } else if (isShown) {
            symbol = value;
        } else {
            symbol = "*";
        }
    }
}

// I am not sure if I am overriding these correctly (hashCode(),equals())

@Override
public int hashCode() {
    return xPos * yPos * symbol.hashCode() * value.hashCode() * 29 + 6;
}

@Override
public boolean equals(Object obj) {
    Cell cell = (Cell) obj;
    return cell.getyPos() == yPos && cell.getxPos() == xPos && cell.isMine() == isMine && cell.getValue().equals(value);
}

@Override
public String toString() {
    return "X: " + xPos + " Y: " + yPos + " Value:" + value;
}

}
\$\endgroup\$
3
\$\begingroup\$

Redundant fields

private final int width;
private final int length;

You do not need these.

    this.width = width;
    this.length = length;

Nor here.

public int getWidth() {
    return width;
}

public int getLength() {
    return length;
}

These could be

public int getWidth() {
    return board[0].length;
}

public int getLength() {
    return board.length;
}

Now you're guaranteed consistency with the board.

        for (int i = 0; i < length; i++) {
            for (int j = 0; j < width; j++) {

can become

        for (int i = 0; i < board.length; i++) {
            for (int j = 0; j < board[i].length; j++) {

Again, this ensures that you never overshoot the board dimensions.

hashCode/equals

// I am not sure if I am overriding these correctly (hashCode(),equals())

@Override
public int hashCode() {
    return width * length * numOfMines + 13;
}

@Override
public boolean equals(Object obj) {
    Board boardObj = (Board) obj;
    return Arrays.deepEquals(boardObj.getBoard(), board);
}

These are not consistent with each other. The rule of thumb is that if two objects are equal, then the hashCode values for both should be equal. If the hashCode values are equal, then the objects should usually be equal.

Example source: Best implementation for hashCode method

Simplest implementation if you are using Java 5 or newer:

public int hashCode() {
    return Arrays.deepHashCode(board);
}

This makes the two methods consistent. Both are based on the values of board.

But what if obj in equals is not a Board?

public boolean equals(Object obj) {
    // if obj is the same object as this, no need to process more
    if (obj == this) {
        return true;
    }

    // if obj is null, then it can't equal this
    // if obj is a different class than this, then they can't be equal
    if (obj == null || obj.getClass() != getClass()) {
        return false;
    }

    Board boardObj = (Board) obj;
    return Arrays.deepEquals(boardObj.getBoard(), board);
}

No need to waste time if we're talking about the same object, not just two potentially equivalent objects.

Now we won't have any exceptions because someone tried to check equality of a Board with a Cell.

Example source: How to override equals method in java

Note that that source offers some more complex examples. For example, it handles the case where you might have some other class extend Board. I didn't do that here, as your current example doesn't need it.

public int hashCode() {
    return xPos * yPos * symbol.hashCode() * value.hashCode() * 29 + 6;
}

So if you're using Java 7 or newer, you can say

public int hashCode() {
    return Objects.hash(yPos, xPos, isMine, value);
}

Now it will be consistent with your equals implementation (which has the same issues as the one for Board, which I won't repeat).

If compiling against an older Java, you could do a custom implementation like

public int hashCode() {
    int result = 6;

    result = 29 * result + yPos;
    result = 29 * result + xPos;
    result = 29 * result + (isMine ? 0 : 1);
    result = 29 * result + ((value == null) ? 0 : value.hashCode());

    return result;
}

Again, note that this uses the same fields as the equals method.

\$\endgroup\$
  • \$\begingroup\$ Thank you for you review, but I really wanted to know your opinion on Game.class. It is the one I'm most unsatisfied with. \$\endgroup\$ – ym123 Aug 8 '17 at 6:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.