1
\$\begingroup\$

This is my Minesweeper JavaFx app. It is working without any weird bugs, at least none that I found. I am not too concerned about the way it looks either, the main reason I post this here is to get an opinion on the design and code. I have gone for a more functional approach but it kinda feels weird, a mid point between functional and object oriented without doing anything right. I would love some input on how to make it more functional. Also please point out any bad habits or patterns.

package minesweeper;

import javafx.application.Application;
import javafx.fxml.FXMLLoader;
import javafx.scene.Scene;
import javafx.scene.layout.AnchorPane;
import javafx.stage.Stage;

import java.io.IOException;

public class Main extends Application{
    public static void main(String... args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) throws IOException{
        AnchorPane root = FXMLLoader.load(getClass().getResource("/minesweeper/minesweeper.fxml"));
        primaryStage.setTitle("Minesweeper");
        primaryStage.setResizable(false);
        primaryStage.setScene(new Scene(root, 800, 800));
        primaryStage.show();
    }
}

package minesweeper;

import javafx.fxml.FXML;
import javafx.fxml.FXMLLoader;
import javafx.scene.control.Button;
import javafx.scene.layout.AnchorPane;

import java.io.IOException;
import java.net.URI;

public class ControllerMenu {
    @FXML public AnchorPane root;
    @FXML public Button ButtonPlay, ButtonHelp, ButtonExit;

    public void play() throws IOException{
        root.getScene().setRoot(FXMLLoader.load(getClass().getResource("/minesweeper/Game.fxml")));
    }

    public void exit(){
        System.exit(0);
    }
}

package minesweeper;

import javafx.scene.control.Button;

class Tile extends Button {
    private boolean flagged, mine;
    private final int x, y;
    private int value;

    Tile(int x, int y){
        this.x = x;
        this.y = y;

        this.setStyle("-fx-font-weight: bold");
        this.setPrefSize(28, 28);
    }

    int getX(){
        return x;
    }

    int getY(){
        return y;
    }

    int getValue(){
        return value;
    }

    void setValue(int n){
        value = n;
    }

    void flag(){
        flagged = !flagged;
    }

    boolean isFlagged(){
        return flagged;
    }

    void setMine(){
        mine = true;
        value = 9;
    }

    boolean isMine(){
        return mine;
    }

    void disable(){
        this.setDisable(true);

        if(mine)
            this.setStyle("-fx-background-color: red");
        else if (value > 0)
            this.setText(value+"");
    }
}

package minesweeper;

import java.util.*;
import java.util.stream.IntStream;

class Board {
    final int ROWS = 25;
    final int COLUMNS = 25;
    final int MINES = 75;
    private final Tile[][] tiles = new Tile[ROWS][COLUMNS];

    Board() {
        makeTiles();
        addMines();
        updateValues();
    }

    Tile getTile(int x, int y) {
        return tiles[x][y];
    }

    private LinkedList<Tile> flatTiles() {
        LinkedList<Tile> flatTiles = new LinkedList<>();
        Arrays.stream(tiles).forEach(t -> flatTiles.addAll(Arrays.asList(t)));
        return flatTiles;
    }

    private void makeTiles() {
        IntStream.range(0, ROWS)
            .parallel()
            .forEach(r -> IntStream.range(0, COLUMNS)
                .parallel()
                .forEach(c -> tiles[r][c] = new Tile(r, c)));
    }

    private void addMines() {
        Random rng = new Random();
        Set<List<Integer>> MineCoordinates = new HashSet<>(MINES);

        while (MineCoordinates.size() < MINES)
            MineCoordinates.add(List.of(rng.nextInt(ROWS), rng.nextInt(COLUMNS)));

        for(List<Integer> x : MineCoordinates)
            tiles[ x.get(0) ][ x.get(1) ].setMine();
    }

    private void updateValues() {
        flatTiles().stream().filter(t -> !t.isMine())
            .forEach(t -> t.setValue((int) adjacent(t).stream()
                .parallel()
                .filter(Tile::isMine)
                .count()));
    }

    List<Tile> adjacent(Tile tile) {
        List<Tile> adjacent = new LinkedList<>();
        int neighbors[] = {-1, 0, 0, -1, -1, -1, -1, 1, 1, -1, 1, 0, 0, 1, 1, 1};

        for(int i=0; i<neighbors.length; i+=2){
            int Newx = tile.getX()+neighbors[i];
            int Newy = tile.getY()+neighbors[i+1];

            if (Newx>=0 && Newx<25 && Newy>=0 && Newy<25)
                adjacent.add(tiles[Newx][Newy]);
        }
        return adjacent;
    }
}

package minesweeper;

import javafx.animation.KeyFrame;
import javafx.animation.Timeline;
import javafx.fxml.FXML;
import javafx.fxml.FXMLLoader;
import javafx.geometry.Insets;
import javafx.scene.control.Label;
import javafx.scene.input.KeyCode;
import javafx.scene.input.KeyEvent;
import javafx.scene.input.MouseButton;
import javafx.scene.layout.AnchorPane;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.Pane;
import javafx.util.Duration;

import java.io.IOException;
import java.util.stream.IntStream;

public class Game {
    @FXML public AnchorPane gameScene;
    @FXML public Pane Container;
    @FXML public Label mines, tiles;
    private final Board board = new Board();
    private int mineCount = 0, tileCount = 0;

    public void initialize(){
        setUpGrid();
        gameScene.addEventHandler(KeyEvent.KEY_PRESSED, e -> {
            if (KeyCode.ESCAPE == e.getCode())
                try {
                    AnchorPane root = FXMLLoader.load(getClass().getResource("/minesweeper/minesweeper.fxml"));
                    gameScene.getScene().setRoot(root);
                }catch (IOException ex){
                    ex.printStackTrace();
                }
        });
        mines.setText("Mines: 0/" + board.MINES);
        tiles.setText("Tiles: 0/" + board.ROWS * board.COLUMNS);
    }

    private void reveal(Tile tile) {
        tile.disable();
        tileCounter();

        if (tile.isMine())
            gameover(false);
        else if (tile.getValue() == 0)
            board.adjacent(tile).stream()
                .filter(e -> !(e.isDisabled() || e.isMine()))
                .forEach(this::reveal);
    }

    private void flag(Tile t){
        mineCounter(!t.isFlagged());
        t.flag();
        t.setText(t.isFlagged() ? "O" : "");
    }

    private void gameover(boolean won){
        try{
            AnchorPane root = FXMLLoader.load(getClass().getResource("/minesweeper/" + (won ? "Victory" : "Loss") + ".fxml"));
            Timeline tl = new Timeline(new KeyFrame(Duration.seconds(2), e -> gameScene.getScene().setRoot(root)));
            tl.play();
        }catch (IOException e){
            e.printStackTrace();
        }
    }

    private void mineCounter(boolean inc){
        mines.setText("Mines: " + (inc ? ++mineCount : --mineCount) + "/" +board.MINES);
    }

    private void tileCounter(){
        tiles.setText("Tiles: "+ ++tileCount + "/" +board.ROWS*board.COLUMNS);
        if(gameWon())
            gameover(true);
    }

    private boolean gameWon(){
        return tileCount == board.ROWS * board.COLUMNS - board.MINES;
    }

    private void setUpGrid(){
        GridPane grid = new GridPane();
        grid.setPadding(new Insets(3));
        grid.setHgap(1);
        grid.setVgap(1);

        IntStream.range(0, board.ROWS)
            .forEach(r -> IntStream.range(0, board.COLUMNS)
                .forEach(c -> grid.add(createTile(r, c), c, r)));

        Container.getChildren().add(grid);
    }

    private Tile createTile(int x, int y){
        final Tile tile = board.getTile(x, y);
        tile.setOnMouseClicked(e -> {
            if(e.getButton()==MouseButton.PRIMARY)
                reveal(tile);
            else
                flag(tile);
        });
        return tile;
    }
}

package minesweeper;

import javafx.fxml.FXML;
import javafx.fxml.FXMLLoader;
import javafx.scene.control.Button;

public class ReplayController {
    @FXML Button replay;

    public void PlayAgain() throws Exception{
        replay.getScene().setRoot(FXMLLoader.load(getClass().getResource("Game.fxml")));
    }
}
\$\endgroup\$
0
\$\begingroup\$

Thanks for sharing your code.

General coding

You declare member variables of the same type in one line. This is an unusual way to do it. It is certainly not a problem as long as you are the only one working on your code. But when You join a team it is important to negotiate that with the others...

Naming

Finding good names is the hardest part in programming. So always take your time to think carefully of your identifier names.

Naming Conventions

It looks like you already know the Java Naming Conventions but you don't strictly follow them.

  • Variables should begin with a lowercase letter but you have quite a lot starting with upper case.

  • methods should start with a verb that points to what will be done in the method. E.g. your method Board.adjacent(Tile tile) should better be named Board.findAdjacentOf(Tile tile).

  • Variables holding a boolean or methods returning a boolean should start with is, has, can or alike.

Functional Programming

FP is about immutable, independend data

Any book about functional programming states that it is all about flow of stateless independend data. The main idea of FP is that you have immutable data having no relationship to each other which you process to create other immutable data. That means in turn that you cannot transcript anything to a functional program that depends on state or where the data have some sort of dependency to each other.

However, one of that books on FP (teaching closure) stated that you create "pearls of FP code* to be integrated in your "ordinary" program.

The problem I have with your code in this respect is that the Tile objects your functional code works on are mutable, that means they have state that changes during the programs runtime. E.g. your methods updateValues() and makeTiles() violate the FP principle of working on and with immutable data. As a rule of thumb: if you cannot replace .forEach() the behavior you want to implement might not be suitable to be solved with a FP approach.

For your program that means that you have parts that look like FP (because you use Java8s "functional API") but they are not.

be consequent with you coding style.

On the other hand you have some places in your code where you miss to use a FP approach. E.g. the adjacent method could be implemented in an FP way like this:

 private static final int X = 0,Y = 1; // avoid "magic numbers"
 private static final int[][] neighborOffsets // does not change during program runtime
    = {{-1, 0}, {0, -1}, {-1, -1}, {-1, 1}, {1, -1}, {1, 0}, {0, 1}, {1, 1}}; // show that these are "pairs", not individual independend values

 private boolean isInGrid(int neighborIndex){ // helper method
    return 0<=neighborIndex && 25> neighborIndex);
 }

 List<Tile> adjacent(Tile tile) {
   return  Straem.of(neighborOffsets)
                 .filter(offset-> isInGrid(tile.getX()+offset[X]) && isInGrid(tile.getY()+offset[Y]))
                 .map(offset-> tiles[tile.getX()+offset[X]][tile.getY()+offset[Y]])
                 .collect(Collectors.toList());
 }

The difference to your "flawed" functional approaches in updateValues() and makeTiles() is that here a new collection is created and the state of the existing data is not changed.

Conclusion

Functional Programming is just another tool in the programmers tool box, but its a very special that is useful only in certain situations. Nails are not useless just because you found a screwdriver next to your hammer...

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ That is actually way more useful than I could have hoped for. I will take to heart each of your points (since all make sense to me) and will rewrite some stuff. Thank you for your time and effort to provide examples. \$\endgroup\$ – Khepu Mar 12 '18 at 4:32

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.