Sudoku-Solver - follow-up

Question

I've now tried to use the suggestions you can find here to improve my Sudoku-Solver.

Here's the updated code:

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import static javax.swing.WindowConstants.EXIT_ON_CLOSE;

public class SudokuSolver {

    public static void main(String[] args) {

        SwingUtilities.invokeLater(SudokuSolver::createGUI);
    }

    private static void createGUI() {
        int[][] board = {
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
            { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
        };
        JFrame frame = new JFrame();
        frame.setSize(800, 700);
        frame.setDefaultCloseOperation(EXIT_ON_CLOSE);
        JPanel panel = new JPanel();
        JPanel subpanel1 = new JPanel();
        subpanel1.setPreferredSize(new Dimension(500,500));
        subpanel1.setLayout( new java.awt.GridLayout( 9, 9, 20, 20 ) );
        JTextField[][] text = new JTextField[9][9];
        Font font = new Font("Verdana", Font.BOLD, 40);
        for(int i = 0; i < 9; i++) {
            for(int j = 0; j < 9; j++) {
                text[i][j] = new JTextField();
                text[i][j].setText("0");
                text[i][j].setEditable(true);
                text[i][j].setFont(font);

                subpanel1.add(text[i][j]);
            }
        }
        JPanel subpanel2 = new JPanel();
        JButton button = new JButton("OK");
        button.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent actionEvent) {
                for(int i = 0; i < 9; i++) {
                    for(int j = 0; j < 9; j++) {
                        String s = text[i][j].getText();
                        board[i][j] = Integer.valueOf(s);
                    }
                }

                boolean solve = solver(board);
                if(solve) {
                    for(int i = 0; i < 9; i++) {
                        for(int j = 0; j < 9; j++) {
                            text[i][j].setText("" + board[i][j]);
                            text[i][j].setEditable(false);
                        }
                    }
                }
                else {
                    JOptionPane.showMessageDialog(null,"Not solvable.");
                }
                button.setVisible(false);

            }
        });

        subpanel2.add(button);
        panel.add(subpanel1, BorderLayout.WEST);
        panel.add(subpanel2, BorderLayout.EAST);
        frame.add(panel);
        frame.setVisible(true);


    }

    //Backtracking-Algorithm
    public static boolean solver(int[][] board) {

        for (int i = 0; i < 9; i++) {

            for (int j = 0; j < 9; j++) {

                if (board[i][j] == 0) {

                    for (int n = 1; n < 10; n++) {

                        if (checkRow(board, i, n) && checkColumn(board, j, n) && checkBox(board, i, j, n)) {

                            board[i][j] = n;
                            if (!solver(board)) {

                                board[i][j] = 0;
                            }
                            else {
                                return true;
                            }
                        }
                    }
                    return false;
                }
            }
        }
        return true;
    }

    public static boolean checkRow(int[][] board, int row, int n) {
        for (int i = 0; i < 9; i++) {
            if (board[row][i] == n) {
                return false;
            }
        }
        return true;
    }

    public static boolean checkColumn(int[][] board, int column, int n) {
        for (int i = 0; i < 9; i++) {
            if (board[i][column] == n) {
                return false;
            }
        }
        return true;
    }

    public static boolean checkBox(int[][] board, int row, int column, int n) {
        row = row - row % 3;
        column = column - column % 3;

        for (int i = row; i < row + 3; i++) {
            for (int j = column; j < column + 3; j++) {
                if (board[i][j] == n) {
                    return false;
                }
            }

        }
        return true;
    }

}

I think there are some improvements already, but I would appreciate any suggestions to further improve the code (especially the GUI).

---

The follow-up question can be found here.

Answer 1

GUI Design Changes

BorderLayout

You are adding subpanels with:

  panel.add(subpanel1, BorderLayout.WEST);
  panel.add(subpanel2, BorderLayout.EAST);

but you declare panel with:

  JPanel panel = new JPanel();

which by default uses FlowLayout. You probably want to explicitly use BorderLayout:

  JPanel panel = new JPanel(new BorderLayout());

and then I find:

  panel.add(subpanel1, BorderLayout.CENTER);
  panel.add(subpanel2, BorderLayout.PAGE_END);

produces a more pleasing layout.

Centred Text

With the BorderLayout, the 9x9 grid layout will expand to fill most of the application window. With larger windows, the left-aligned text fields look wrong, so instead, add:

  text[i][j].setHorizontalAlignment(JTextField.CENTER);

Grid Gaps

At this point, I removed the GridLayout hgap and vgap, and removed the preferred size for subpanel1:

  JPanel subpanel1 = new JPanel(new GridLayout(9, 9));

GUI Code Refactoring

Member variables

The createGUI() method is a little large; it contains the event handler for the button. Let's move that out into its own function. Since it will need access to the text[i][j] and button, let's move those into members of a SudokuSolver object. Obviously, we'll create need to create a SudokuSolver object, so let's use invokeLater to create the object, and build the GUI inside the constructor.

public class SudokuSolver {

    public static void main(String[] args) {
        SwingUtilities.invokeLater(SudokuSolver::new);
    }

    private final JTextField[][] text;
    private final JButton button;

    SudokuSolver() {
        JFrame frame = new JFrame();
        ...
        text = new JTextField[9][9];
        ...
        button = new JButton("OK");
        button.addActionListener(this::solveBoard);
        ...
        panel.add(subpanel1, BorderLayout.CENTER);
        panel.add(subpanel2, BorderLayout.PAGE_END);
        frame.add(panel);
        frame.setVisible(true);
    }

    private void solveBoard(ActionEvent action_event) {
        ...
    }

    ...
}

The Board

The createGUI() method had a board matrix which was explicitly initialized to a 9x9 grid of 0's. The creation of the GUI didn't use this board at all; it was used by the actionPerformed handler. So it does not need to be included in the constructor's GUI creation code. It can be created as a local variable in the solveBoard() method.

    private void solveBoard(ActionEvent action_event) {
        int board[][] = new int[9][9];

        for(int i = 0; i < 9; i++) {
            for(int j = 0; j < 9; j++) {
                board[i][j] = Integer.valueOf(text[i][j].getText());
            }
        }

        ...
    }

Hiding UI elements

When you make something invisible, the entire UI may need to be updated, as components may grow to use the newly vacated space. It is usually preferable to disable components, instead of making them invisible, when they are no longer needed or appropriate:

        button.setEnabled(false);

Quality of Life Improvements

Suggestions for improvements

Input Validation

What happens if the user enters a bad input for one of the cells, and presses "OK"? The program might crash, which is usually unacceptable behaviour for a GUI application. The user might not even see a console message explaining why the crash occurred!

What if the user enters bad, but valid input, like "10" or "-1" into a cell? The solver won't have any problem finding values that work to solve the puzzle, but does it make sense to even attempt solving it?

Perhaps the "OK" button should only be enabled if all the cells contain only a single digit, and disabled otherwise?

Retry

After solving, or attempting to solve a puzzle, what can the user do? Only close the application. They can't reset the puzzle. What if they made a mistake, and want to change something? They have to reenter all the givens.

This is most apparent if "Not solvable." is displayed. None of the user inputs have been changed, all the cells are still editable, but the "OK" button to solve the puzzle is no longer available? Then the user see "Oh, that cell was supposed to be a 7, not a 1" ... and while they can change the cell value, they can't reattempt the solution! They have to relaunch the application, and enter the values again. Or you could be nice and in this one case, you could leave the "OK" available to try again.

How about a "Reset" button, which is enabled after a successful solve, which removes the solved values and reenables all the input cells?

Uniqueness

Can you tell if there are multiple solutions?

Other Review Comments

Naming

button, text are poor variable names. solve_button would be a little clearer if there was more than one button. Similarly, cell_text_field might be better than text, but perhaps a little verbose; how about cell_tf? Or tfCell if you like Hungarian notation.

subpanel1 could be called grid_panel, and subpanel2 could be called button_panel.