Beginner Minesweeper game

I wrote a basic Minesweeper game for practice. It consist 3 classes:

• Main - only as launcher

• Cell - controling behavior of single cell

• Board - coordinating behavior of cells.

The hardest part for me, was a determining the value of single cells (how many mines is around it), and firing a chain reaction, if some cell was empty(value = 0) - it check closest cells, if they have value, it just reveal it, if they are empty it repeats process on them. So

I wonder if my solution is reasonable (it works, but it look scary), and I would be great, if someone would review it.

These are two methods in Board Class: setCellValues() and scanForEmptyCells(). However I will be gratefull for any commants and advices, also concenring OOP, code structure and style.

I post basic working GUI version, if someone would like to run it.

Main Class:

public class Main {
    public static void main(String[] args){
        Board board = new Board();
        board.setBoard();
    }
}

Board Class:

```
import javax.swing.*;
import java.awt.*;
import java.util.ArrayList;

public class Board {
private Cell[][] cells;
private int cellID = 0;
private int side = 8;
private int limit = side-2;

public void setBoard(){
JFrame frame = new JFrame();
frame.add(addCells());

plantMines();
setCellValues();

frame.pack();
frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
frame.setVisible(true);
}

public JPanel addCells(){
JPanel panel = new JPanel(new GridLayout(side,side));
cells = new Cell[side][side];
for(int i = 0; i loc = generateMinesLocation(10);
for(int i : loc){
getCell(i).setValue(-1);
}
}
/Choose rendom places for mines/
public ArrayList generateMinesLocation(int q){
ArrayList loc = new ArrayList();
int random;
for(int i = 0; i=1 && cells[i][j-1].getValue() == -1) cells[i][j].increment

Some simple pointers to get things started...

Magic numbers

You have hard-coded your side, limit (how is it used, and why does it default to side - 2?), the number of mines, the dimensions of your buttons and the meaning of a Cell's value to -1 when it is a mine... these should either be extracted out as constants, or documented clearly to show how they are being used.

GUI and usability (UX)

Lumping four UX-related suggestions together:

• Your game is missing the fun of minesweeper: a timer and the ability to flag mines!

• Also, it will be nice if you had at least a reset button, if not a button to change the number of mines. :)

• Oh yeah, your expansion logic doesn't seem to expand to also include the numbered cells... It only expands the empty ones, leaving the user to have to manually click on the surrounding numbered cells. This diminishes the playing quality somewhat.

• Following from the earlier section, your Dimensions(20, 20) can be a bit small for high-DPI displays, it will be nice if you either went with higher values, or look for non-pixel-based scaling alternatives... Here's a related StackOverflow link I found that may be of interest:

Side-note: The older Minesweeper for Windows avoids triggering a mine on the first click, it'll be... nice(?) if you can replicate that too. ;)

Other minor code stuff

• Variable declarations should be done with the interface, i.e. List loc = ... instead of ArrayList loc = ...

• Board.getID() can just be return cellID++, since that is a post-increment operator. That means (roughly) after the variable is called, the value before the increment is returned to the caller, but the value after the increment is written to the variable for the next usage. It is not that hard to understand as long as you realize the difference between pre- and post-increment operators.

• This could be just my personal preference, but I'm not fond of the idea of a somewhat complicated ActionListener implementation that knows a Board and also generates JButtons... I have a feeling you might be able to have a generic listener that acts on a pair of (x,y) button coordinates.

• And that nest of if statements... looks ripe for refactoring, but maybe I'll take a more in-depth look the next time.

Code Review: Part Deux (fortified with a bit of Java 8 magic features)

Starting your board

If you can rename setBoard() as getBoard() and change its return type as a JFrame, the following approach is what I believe the more conventional form of starting a Swing application:

public static void main(String[] args) {
    SwingUtilities.invokeLater(() -> new Board().getBoard());
}

Separating the creation of GUI elements and Cells

addCells() is doing both the creating of GUI elements and the initialization of your side * side Cell-array. It will be better if (the renamed) getBoard(), which calls addCells(), is only concerned with creating the GUI layer, and that instantiating your Board class actually initializes the Cells and the mines for us. This means that any Board instances are properly initialized and ready to be played, which is arguably more 'object'-like. Applying these points will result in the following code:

public Board() {
    cells = new Cell[SIDE][SIDE];
    IntStream.range(0, SIDE).forEach(i -> {
        IntStream.range(0, SIDE).forEach(j -> cells[i][j] = new Cell(this));
    });
    init();
}

private void init() {
    plantMines();
    setCellValues();
}

• I have taken the liberty of turning side into a static final SIDE field.

• Will explain init() later...

Iterating through the cells array

Based on my current understanding, a simpler way of iterating through our 2D array is to create a helper method that accepts a Consumer operation in Java 8.

private void forEach(Consumer consumer) {
    Stream.of(cells).forEach(row -> Stream.of(row).forEach(consumer));
}

This allows us to further simplify most of our code afterwards. The first method to be simplified is our addCells() method, provided the separation of concerns described in the earlier section is done.

private JPanel addCells() {
    JPanel panel = new JPanel(new GridLayout(SIDE, SIDE));
    forEach(cell -> panel.add(cell.getButton()));
    return panel;
}

Cell implementation and coupling

As mentioned in my original answer, I am not fond of Cell implementing the ActionListener interface, and I could be biased. My take on this is to give each JButton generated by the Cell a listener that calls its checkCell() method instead.

Cell(Board board) {
    this.board = board;
    button = new JButton();
    button.addActionListener(listener -> { this.checkCell(); });
    ...
}

As pointed out by @Simon, it'll be better have methods that directly represents the exact actions we want on a Cell, i.e. to setMine(), or check isMine(), or the more straightforward isChecked() as opposed to `is

StackExchange Code Review Q#88636, answer score: 11