3

I have been working on a sudoku puzzle generator in java, I wrote this class to generate the puzzle but it is not correctly generating the puzzle. Here is an example of what I am getting:

puzzle

As you can see this is not a valid sudoku solution. But looking at my code, I don't understand why it is not generating a valid puzzle. Can someone explain why this does not work correctly?

package sudoku;

import java.util.Random;

public class Puzzle {

    // number generator
    Random gen = new Random();

    // 9x9 puzzle
    int puzzle[][] = new int[9][9];

    public int[][] generate() {

        // add each number to the board
        for (int x = 0; x < 9; x++) {
            for (int y = 0; y < 9; y++) {

                boolean isValid = false;

                // keep generating new numbers until a valid number is found
                while (isValid == false) {

                    // generate random number 1-9
                    int num = gen.nextInt(9) + 1;

                    // check if number is valid
                    if (checkRow(num, x) == true || checkCol(num, y) == true
                            || checkSection(num, x, y) == true) {

                        // add number to the board
                        puzzle[x][y] = num;

                        // exit loop
                        isValid = true;
                    }
                }
            }
        }

        return puzzle;
    }

    // check each element of the row for num, if num is found return false
    private boolean checkRow(int num, int row) {

        boolean valid = true;
        for (int i = 0; i < 9; i++) {
            if (puzzle[row][i] == num) {
                valid = false;
                break;
            }
        }

        return valid;
    }

    // check each element of the column for num, if num is found return false
    private boolean checkCol(int num, int col) {

        boolean valid = true;
        for (int i = 0; i < 9; i++) {
            if (puzzle[i][col] == num) {
                valid = false;
                break;
            }
        }

        return valid;
    }

    // check each element of the section for num, if num is found return false
    private boolean checkSection(int num, int xPos, int yPos) {

        int[][] section = new int[3][3];
        section = getSection(xPos, yPos);

        boolean valid = true;
        for (int i = 0; i < 3; i++) {
            for (int j = 0; j < 3; j++) {
                if (section[i][j] == num) {
                    valid = false;
                    break;
                }
            }
        }

        return valid;
    }

    // return the 3x3 section the given coordinates are in
    private int[][] getSection(int xPos, int yPos) {

        int xIndex = 0;
        int yIndex = 0;
        int[][] section = new int[3][3];

        // get x index
        if (xPos == 0 || xPos == 3 || xPos == 6) {
            xIndex = xPos;
        } else if (xPos == 1 || xPos == 4 || xPos == 7) {
            xIndex = xPos - 1;
        } else if (xPos == 2 || xPos == 5 || xPos == 8) {
            xIndex = xPos - 2;
        }

        // get y index
        if (yPos == 0 || yPos == 3 || yPos == 6) {
            yIndex = yPos;
        } else if (yPos == 1 || yPos == 4 || yPos == 7) {
            yIndex = yPos - 1;
        } else if (yPos == 2 || yPos == 5 || yPos == 8) {
            yIndex = yPos - 2;
        }

        int i = 0;
        int j = 0;
        // extract section from puzzle
        for (int x = xIndex; x < 3; x++) {
            for (int y = yIndex; y < 3; y++) {
                section[x][y] = puzzle[i][j];
                i++;
            }
            j++;
        }

        return section;

    }
}
Petefic
  • 657
  • 6
  • 14
  • 31
  • 1
    Your class is completely procedurally written, with nary any trace of object orientation. You may find that your algorithm is easier to troubleshoot if you modularized it into objects, each its its own methods to test and construct itself. The nouns in your project are Puzzle, Sections, Squares; perhaps Column and Row and Digit. Those sound like classes. Instead of debugging an enormous monolithic procedure, it would be easier to debug the behavior of individual modules. – scottb Jun 15 '13 at 01:00
  • 1
    It is not error but in `if (someCondition == true)` the `==true` part is redundant, simple `if (someCondition)` is enough. – Pshemo Jun 15 '13 at 01:00

2 Answers2

5

It's not enough that a row, column or section are valid. They ALL MUST be valid. So, change this line:

if (checkRow(num, x) == true || checkCol(num, y) == true || checkSection(num, x, y) == true) 

with

if (checkRow(num, x) == true && checkCol(num, y) == true && checkSection(num, x, y) == true) 

or just simple

if (checkRow(num, x) && checkCol(num, y) && checkSection(num, x, y)) {
darijan
  • 9,725
  • 25
  • 38
2

There might be additional errors, but this line is clearly wrong:

if (checkRow(num, x) == true || checkCol(num, y) == true
                        || checkSection(num, x, y) == true) {

You should use && (boolean AND) here instead of || (boolean OR).

schnaader
  • 49,103
  • 10
  • 104
  • 136