6
\$\begingroup\$

I wrote out a Minesweeper clone for a compsci project. I'm concerned a lot about code efficiency - any suggestions on how I could streamline my code to make it more efficient?

This is the repo for it, if you want to compile/run the program.

MineHandler.java

import java.util.Scanner;
import javax.swing.JFrame;
import javax.swing.JPanel;
import java.awt.BorderLayout;
import javax.swing.BoxLayout;
import javax.swing.*;
import java.awt.Image;
import java.awt.Component;

import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.event.MouseAdapter;

public class MineHandler extends JFrame{
  protected Minesweeper obj; //has the "board"
  private JPanel god; //minesweeper layout design
  private JPanel menu; //menu layout design (preceding god)
  private JPanel top, bottom; //top and bottom stuff on the board

  private ImageIcon cover;

  private JButton easy, medium, hard, custom; //button layout
  private JButton menuReturn;
  private JLabel text, image, difficulty;
  public static void main(String[] args){
    MineHandler crystal = new MineHandler();
  }
  public MineHandler(){

    obj = new Minesweeper();
    menuOpener();
  }

  //you'll need to clean up the UI in a bit
  public void menuOpener(){ //brings up the main menu
    setTitle("Minesweeper");
    setSize(300,400);
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    setResizable(false);

    menu = new JPanel();
    menu.setLayout(new BoxLayout(menu, BoxLayout.Y_AXIS));


    text = new JLabel();
    text.setText("Minesweeper");
    text.setAlignmentX(Component.CENTER_ALIGNMENT);

    cover = new ImageIcon("../img/icon.png");
    image = new JLabel(cover);
    image.setAlignmentX(Component.CENTER_ALIGNMENT);

    easy = new JButton();
    easy.setText("Easy");
    easy.addMouseListener(new MouseAdapter(){
      public void mouseClicked(MouseEvent e){
        obj.easy();
        obj.buildInit();
        difficulty = new JLabel();
        difficulty.setText("EASY Mode: Minesweeper");
        buildGui();
      }
    });
    easy.setAlignmentX(Component.CENTER_ALIGNMENT);

    medium = new JButton();
    medium.setText("Medium");
    medium.addMouseListener(new MouseAdapter(){
      public void mouseClicked(MouseEvent e){
        obj.medium();
        obj.buildInit();
        setSize(400,400);
        difficulty = new JLabel();
        difficulty.setText("MEDIUM Mode: Minesweeper");
        buildGui();
      }
    });
    medium.setAlignmentX(Component.CENTER_ALIGNMENT);

    hard = new JButton();
    hard.setText("Hard");
    hard.addMouseListener(new MouseAdapter(){
      public void mouseClicked(MouseEvent e){
        obj.hard();
        obj.buildInit();
        setSize(1000,1000);
        difficulty = new JLabel();
        difficulty.setText("HARD Mode: Minesweeper");
        buildGui();
      }
    });
    hard.setAlignmentX(Component.CENTER_ALIGNMENT);

    custom = new JButton();
    custom.setText("Custom");
    custom.addMouseListener(new MouseAdapter(){
      public void mouseClicked(MouseEvent e){
        System.out.println("custom");
      }
    });
    custom.setAlignmentX(Component.CENTER_ALIGNMENT);

    menu.add(text);
    menu.add(image);
    menu.add(easy);
    menu.add(medium);
    menu.add(hard);
    menu.add(custom);

    add(menu);

    setVisible(true);
  }
  public void buildGui(){
    getContentPane().removeAll();
    getContentPane().repaint();

    System.out.println("Build Gui intiated");


    god = new JPanel(new BorderLayout());
    //board = obj.buildBoard();

    top = new JPanel();

    top.add(difficulty);

    bottom = new JPanel();
    menuReturn = new JButton();
    menuReturn.setText("Return to Menu");
    menuReturn.addMouseListener(new MouseAdapter(){
      public void mouseClicked(MouseEvent e){
        getContentPane().removeAll();
        getContentPane().repaint();

        menuOpener();
        validate();
      }
    }); //add mouse listener



    bottom.add(menuReturn);


    god.add(obj.board, BorderLayout.CENTER);
    god.add(top, BorderLayout.NORTH);
    god.add(bottom, BorderLayout.SOUTH);
    add(god);
    validate();
  }

}

//begin to implement flag operations

Minesweeper.java

import java.util.Random;
import javax.swing.JPanel;
import javax.swing.*;
import java.awt.GridLayout;
public class Minesweeper{
  /*
  0: empty space:
  1 ~ 8 : bomb next to it
  9: key bomb (same bomb for successfully uncovered bombs in fail and pass)
  10: flag
  11: unused
  12: incorrect flags
  13: the bomb user clicked in loss
  14: covered space
  */
  private int length, width; //sets the length and width
  private Cell[][] array;
  protected int[][] key;
  protected int[][] userArray;
  private int[][] chainReactionSet;
  private int bombs;
  private Random box;
  protected JPanel board;
  //add mouse event here?
  public Minesweeper(){

    box = new Random();
  }

  public void easy(){
    length = 10;
    width = 10;
    bombs = 10;
  }

  public void medium(){
    length = 16;
    width = 16;
    bombs = 40;
  }

  public void hard(){
    length = 30;
    width = 35;
    bombs = 99;
  }
  public void buildInit(){
    bombInit();
    buildBoard();
    chainReactionSet = new int[key.length][];
    for(int i = 0; i < key.length; i++){
      chainReactionSet[i] = key[i].clone();
    }
  }
  public void bombInit(){
    key = new int[length][width];
    for(int i = 0; i< bombs; i++){
      key[box.nextInt(length)][box.nextInt(width)] = 9;
    }
    toString(key);
    numberSet();
    genUserArray();
  }
  //needs fixing
  public void winValidate(){
    //conditions: ALL flags must be CORRECTLY paired with ALL the bombs AND all blank spaces must be cleared out
    boolean isWin = true;
    for(int i = 0 ; i < userArray.length; i ++){
      for(int a = 0 ; a < userArray[0].length; a++){
        if(array[i][a].isMine == true && array[i][a].isFlagged == false){ //checks to see all bombs are paired with correct flags
          isWin = false;

        }

        else if(array[i][a].isFlagged == false && array[i][a].isEnabled == true){
          isWin = false;

        }
      }
    }
    if(isWin == true){
      win();
    }

  }
  public void win(){
    for(int i = 0; i < key.length; i++){
      for(int a = 0; a < key[0].length; a++){
        array[i][a].isEnabled = false;
      }
    }
    JOptionPane.showMessageDialog(null, "You won");
  }



  private void numberSet(){

    for (int i = 0; i < key.length; i++){
      for (int a = 0; a < key[0].length; a++){
        //If it Is not a mine
        if (key[i][a] == 0){
          int count = 0;
          //Position of cells around key[i][a] relative to key[i][a]
          for (int x = -1; x < 2; x++){
            for (int y = -1; y < 2; y++){
              //Storing x and y test points
              int testX = i + x;//testX = i-1, i, and i+1
              int testY = a + y;//testY = i-1, y, and y+1
              //If the testX and testY values are within the range of the array
              if ((testX >= 0 && testX < length) && (testY >= 0 && testY < width)){
                //If there is a mine
                if (key[testX][testY] == 9){count++;}

              }
            }
          }
          key[i][a] = count;


        }

      }
    }
  }

  private void genUserArray(){
    userArray = new int[length][width];
    for(int i = 0; i < userArray.length; i++){
      for(int a = 0; a < userArray[0].length; a++){
        userArray[i][a] = 14;
      }
    }
  }

  private JPanel buildBoard(){

    board = new JPanel(new GridLayout(length,width));
    array = new Cell[length][width];
    for(int i = 0; i < userArray.length; i++){ //set all the arrays in the cell array to 14
      for(int a =0 ; a < userArray[0].length; a++){
        array[i][a] = new Cell(i,a,this);
        array[i][a].changeIcon(userArray[i][a]);
        if(key[i][a] == 9){array[i][a].isMine = true;}
        //System.out.println("here");
        board.add(array[i][a].button);
      }
    }
    return board;
  }

  public void chainReaction(int col, int row){
    chainReactionSet[col][row] = -2;
    if(key[col][row] != 0){
      array[col][row].updateNum(key[col][row]);
      array[col][row].isEnabled = false;
      array[col][row].isNumbered = true;
    }
    else if(key[col][row] == 0){
      array[col][row].changeIcon(key[col][row]);
      array[col][row].isEnabled = false;
      array[col][row].isBlank = true;
      for (int x = -1; x < 2; x++){
        for (int y = -1; y < 2; y++){
          //Storing x and y test points
          int testX = col + x;//testX = i-1, i, and i+1
          int testY = row + y;//testY = i-1, y, and y+1
          //If the testX and testY values are within the range of the array
          if ((testX >= 0 && testX < length) && (testY >= 0 && testY < width)){
            //If there is a mine
            if (key[testX][testY] != 9 && chainReactionSet[testX][testY] != -2){


              chainReaction(testX,testY);

            }

          }
        }

      }
    }


  }
  public static void toString(int[][] arr){
    for(int a[] : arr){
      for(int b : a){
        System.out.print(b + " ");
      }
      System.out.println();
    }
  }
  public void fail(){
    for(int i = 0; i < userArray.length; i++){
      for(int a = 0; a < userArray[0].length; a++){
        array[i][a].isEnabled = false;
        if(userArray[i][a] == 13){
          continue;
        }
        if(userArray[i][a] == 10 && key[i][a] != 9){
          array[i][a].changeIcon(12);
        }
        else if(userArray[i][a] == 10 && key[i][a] == 9){
          continue;
        }
        else if(key[i][a] == 0 || key[i][a] == 9){
          array[i][a].changeIcon(key[i][a]);
        }
        else{
          array[i][a].updateNum(key[i][a]);
        }
      }
    }
    JOptionPane.showMessageDialog(null, "You lost");
  }
}

Cell.java

import javax.swing.*;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.event.MouseAdapter;
import java.awt.Insets;
public class Cell{
  private ImageIcon image;
  protected int status, col,row; //status records the current STATUS OF THE CELL
  protected JButton button;
  private Minesweeper miney;
  protected boolean isEnabled;
  protected boolean isMine;
  protected boolean isFlagged;
  protected boolean isNumbered;
  protected boolean isBlank;
  //protected Mouse mouseBox;
  public Cell(int i,int a, Minesweeper miney){
    isEnabled = true;
    isFlagged = false;
    isBlank = false;
    col = i;
    row = a;
    this.miney = miney;
    //mouseBox = new Mouse(i,a);
    button = new JButton();
    //button.addMouseListener(mouseBox);
    button.addMouseListener(new MouseAdapter(){
      public void mouseClicked(MouseEvent e){

        if(e.getButton() == MouseEvent.BUTTON1){
          if(isEnabled){
            if(miney.userArray[col][row] == 10){}
            else if(miney.key[col][row] == 9){
              miney.userArray[col][row] = 13;
              changeIcon(13);
              miney.fail();
            }
            else if(miney.key[col][row] == 0){
              miney.chainReaction(col,row);

            }

            else{
              if(isEnabled){
                miney.userArray[col][row] = miney.key[col][row];
                updateNum(miney.key[i][a]);
                isEnabled = false;
                isNumbered = true;
              }

            }
            miney.winValidate();
          }

        }
        else if(e.getButton() == MouseEvent.BUTTON3){
          if(isEnabled){
            if(miney.userArray[col][row] == 10){
              miney.userArray[col][row] = 14;
              status = 14;
              changeIcon(14);
              isFlagged = false;
            }
            else if(miney.userArray[col][row] == 14){
              miney.userArray[col][row] = 10;
              status = 10;
              changeIcon(10);
              isFlagged = true;
            }
            miney.winValidate();
          }

        }

      }
    });

  }
  public void updateNum(int num){
    status = num;
    button.setIcon(null);
    String text = Integer.toString(num);
    System.out.println(num);
    button.setText(text);

    button.setBorder(null);
    button.setBorderPainted(false);
    button.setMargin(new Insets(0,0,0,0));
    button.setEnabled(false);
  }
  public void changeIcon(int img){ //adds the image to the button
    status = img;
    String directory = "../img/" + status + "box.png";
    ImageIcon box = new ImageIcon(directory);


    button.setIcon(box);

  }
  public JButton addCell(){//adds the cell to the larger array
    return button;
  }
}
\$\endgroup\$
1
\$\begingroup\$

I wrote out a Minesweeper clone for a compsci project. I'm concerned a lot about code efficiency

What is your definition of "code efficiency"?
I assume it is performance.

In that case there is a name for this: premarture optimization.

Java is especially designed to save the programmer from low level performacne considerations most of the time. Therefore your major concern should be redability and maintainability of your code (unless you have proven a performence problem with your code).

any suggestions on how I could streamline my code to make it more efficient?


When looking at your code example I'd suggest the following:

readability

Your code uses magic numbers wich you describe in a comment.

Instead of that comment declare public constants which provides neaningfull names:

 public static final int EMPTY = 0;
 public static final int FLAG = 10;

Or even better an enum in its own class) The probem here is that you "reuse" the same variable for different semantics: values 1 to 8 are "the neigbours state" all other values are "the cells own state". If we put this aside the enum would be like this

public enum CellState{
  EMPTY_COVERED, BOMB_COVERED, EMPTY_UNCOVERED, BOMB_UNCOVERED
}

separation of concerns

Your class Cell looks pretty much like a model class providing data handling. But this should not know about GUI classes like JButton or controller classes like ActionListener

This is how I would do this part (only display, no covering/uncovering):

Cell.java
class Cell {
    Collection<Cell> neigbours = new HashSet<>();
    private final boolean isBomb;

    public Cell(boolean isBomb) {
        this.isBomb = isBomb;
    }

    public void addNeigbour(Cell other) {
        neigbours.add(other);
    }

    @Override
    public String toString() {
        if (isBomb)
            return "X";

        int neigbourBombCount = countBombs(neigbours);
        if (0 == neigbourBombCount)
            return " ";
        return String.valueOf(neigbourBombCount);
    }

    private int countBombs(Collection<Cell> neigbours2) {
        int neigbourBombCount = 0;
        for (Cell cell : neigbours2) {
            if (cell.isBomb)
                neigbourBombCount++;
        }
        return neigbourBombCount;
    }
}
MineSweeper.java
public class MineSweeper {
    public static void main(String[] args) {
        Cell[][] gameField = configureFields();
        JTable jTable = prepareDisplay(gameField);
        JOptionPane.showMessageDialog(null, jTable);
    }

    private static Cell[][] configureFields() {
        Cell[][] gameField = initializeCellArray();
        setCellNeigbours(gameField);
        return gameField;
    }

    private static void setCellNeigbours(Cell[][] gameField) {
        for (int i = 0; i < gameField.length; i++) {
            Cell[] fieldRow = gameField[i];
            for (int j = 0; j < fieldRow.length; j++) {
                Cell currentCell = fieldRow[j];
                setCellNeigbours(gameField, i, fieldRow, j, currentCell);
            }
        }
    }

    private static Cell[][] initializeCellArray() {
        final int bombLikelyness = 7;
        Cell[][] gameField = new Cell[50][30];
        for (int i = 0; i < gameField.length; i++) {
            Cell[] fieldRow = gameField[i];
            for (int j = 0; j < fieldRow.length; j++) {
                gameField[i][j] = new Cell(0 == new Random().nextInt(bombLikelyness) % bombLikelyness);
            }
        }
        return gameField;
    }

    private static void setCellNeigbours(Cell[][] gameField, int i, Cell[] fieldRow, int j, Cell currentCell) {
        if (0 < i && 0 < j)
            currentCell.addNeigbour(gameField[i - 1][j - 1]);
        if (0 < j)
            currentCell.addNeigbour(gameField[i][j - 1]);
        if (0 < i)
            currentCell.addNeigbour(gameField[i - 1][j]);
        if (gameField.length > i + 1 && fieldRow.length > j + 1)
            currentCell.addNeigbour(gameField[i + 1][j + 1]);
        if (gameField.length > i + 1)
            currentCell.addNeigbour(gameField[i + 1][j]);
        if (fieldRow.length > j + 1)
            currentCell.addNeigbour(gameField[i][j + 1]);

        if (0 < i && fieldRow.length > j + 1)
            currentCell.addNeigbour(gameField[i - 1][j + 1]);
        if (gameField.length > i + 1 && 0 < j)
            currentCell.addNeigbour(gameField[i + 1][j - 1]);
    }


    private static JTable prepareDisplay(Cell[][] gameField) {
        DefaultTableModel defaultTableModel = new DefaultTableModel() {
            @Override
            public int getColumnCount() {
                return gameField.length;
            }

            @Override
            public int getRowCount() {
                return gameField[0].length;
            }

            @Override
            public Object getValueAt(int row, int column) {
                return gameField[column][row];
            }
        };

        JTable jTable = new JTable(defaultTableModel);
        jTable.setPreferredSize(new Dimension(15*gameField.length, 20*gameField[0].length));

        jTable.setDefaultRenderer(Object.class, new DefaultTableCellRenderer() {

            @Override
            public Component getTableCellRendererComponent(JTable table, Object value, boolean isSelected, boolean hasFocus,
                    int row, int column) {
                 JLabel rendererComponent = (JLabel) super.getTableCellRendererComponent(table, value, isSelected, hasFocus, row, column);
                 rendererComponent.setHorizontalAlignment(CENTER);
                 switch (value.toString()) {
                case "X":
                     rendererComponent.setBackground(Color.RED);
                    break;
                case " ":
                     rendererComponent.setBackground(Color.GREEN);
                    break;

                default:
                     rendererComponent.setBackground(Color.WHITE);
                    break;
                }
                return rendererComponent;
            }
        });
        return jTable;
    }
}
\$\endgroup\$
  • \$\begingroup\$ If Cell.java doesn't handle user input (e.g. if a cell gets clicked), does that mean that role would go to Minesweeper.java? Any suggestions as to manage user input from Minesweeper.java's view for each of the cells? \$\endgroup\$ – Brian Lee Jan 7 '17 at 0:41
  • \$\begingroup\$ @BrianLee "If Cell.java doesn't handle user input (e.g. if a cell gets clicked), does that mean that role would go to Minesweeper.java?" In your implementation the handling of user input should go the MineHandler since this is the user interface in your implementation. MineHandler should display the content of the Cells and translate user input into call to class Minesweeper to change the values in Cell. Your class Minesweeper is the controller working with the Model (Cell). \$\endgroup\$ – Timothy Truckle Jan 7 '17 at 10:19

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.