0
votes

Still working on my sudoku solver, I have once again run into some trouble. I have gotten the sudoku solver to work, however whenever I attempt to solve a really "hard" sudoku board, my solver tells me there are no possible solutions, due to a stack overflow error. And yes, I know for a fact that these boards DO have a solution.

I am using the brute force algorithm -- I start at square one (or [0][0] if you prefer) and insert the first legal value. I then do a recursive call to the next square, and so on. If my function has not gone through the entire board, and finds there are no possible legal values, it moves to the previous square and attempts to increment the value there. If that fails, it moves further back. If it ends up at square one with no possible solutions, it exits.

I admit my code is not pretty and probably quite inefftective, but please keep in mind that I am a first year student trying to do my best. I don't mind comments on how to effectivize my code though :)

For squares without a final predefined number, here's the solver function:

public void fillRemainderOfBoard(Square sender){

    try {

        while(true){
            if(currentPos > boardDimension){
                if(previous != null){
                    this.setNumrep(-1);
                    this.setCurrentPos(1);
                    previous.setCurrentPos(previous.getCurrentPos()+1);
                    previous.fillRemainderOfBoard(this);
                    return;
                } else if(sender == this){
                    this.setCurrentPos(1); 
                } else {
                    break;
                }
            }
            if((thisBox.hasNumber(currentPos)) || (thisRow.hasNumber(currentPos)) || (thisColumn.hasNumber(currentPos))){
                currentPos++;
            } else {
                this.setNumrep(currentPos);
                currentPos++;
                break;
            }
        }

        if(this != last){
            next.setNumrep(-1);
            next.fillRemainderOfBoard(this);
        } else {

            return;
        }
    } catch (StackOverflowError e){
        return;
    }
}

In my code, the numrep value is the value that the square represents on the board. The currentPos variable is a counter that starts at 1 and increments until it reaches a legal value for the square to represent.

For squares with a predefined number, here's the same function:

public void fillRemainderOfBoard(Square sender){
    if((sender.equals(this) || sender.equals(previous)) && this != last){
        System.out.println(next);
        next.fillRemainderOfBoard(this);
    } else if (sender.equals(next) && this != first) {
        previous.fillRemainderOfBoard(this);
    } else {
        System.out.println("No possible solutions for this board.");
    }
}

Now, my problem is, like I said, that the function DOES solve sudokus very well. Easy ones. The tricky sudokus, like those with many predefined numbers and only a single solution, just makes my program go into a stack overflow and tell me there are no possible solutions. I assume this indicates I am missing something in the terms of memory management, or the program duplicating objects which I call in my recursive functions, but I do not know how to fix them.

Any help is greatly appreciated! Feel free to pick on my code too; I'm still learning (oh, aren't we always?).

___________________EDIT________________

Thanks to Piotr who came up with the good idea of backtracking, I have rewritten my code. However, I still cannot get it to solve any sudoku at all. Even with an empty board, it gets to square number 39, and then returns false all the way back. Here is my current code:

public boolean fillInnRemainingOfBoard(Square sender){
    if(this instanceof SquarePredef && next != null){
        return next.fillInnRemainingOfBoard(this);
    } else if (this instanceof SquarePredef && next == null){
        return true;
    }

    if(this instanceof SquareNoPredef){
        currentPos = 1;
        if(next != null){
            System.out.println(this.index);
            for(; currentPos <= boardDimension; currentPos++){
                if((thisBox.hasNumber(currentPos)) || (thisRow.hasNumber(currentPos)) || (thisColumn.hasNumber(currentPos))){
                    continue;
                } else {
                    System.out.println("Box has " + currentPos + "? " + thisBox.hasNumber(currentPos));
                    this.setNumrep(currentPos);
                    System.out.println("Got here, square " + index + " i: " + numrep);
                }

                if(next != null && next.fillInnRemainingOfBoard(this)){
                    System.out.println("Returnerer true");
                    return true;
                } else {

                }
            }
        }
        if(next == null){
            return true;
        }
    }
    System.out.println("Returning false. Square: " + index + " currentPos: " + currentPos);
    return false;
}

I had to complicate things a bit because I needed to check whether the current object is the last. Hence the additional if tests. Oh, and the reason I am using boardDimension is because this sudoku solver will solve any sudoku -- not just those 9 by 9 sudokus :)

Can you spot my error? Any help is appreciated!

1
Get some ideas from here.OldCurmudgeon
You may already realize this, but the StackOverflow is caused by too much unended method calls. This is usually caused by a recursive algorithm that keeps recalling and recalling and never complete the calls. You can try changing a part of your algorithm from recursion to iteration. The solution pointed in the above comment suggests backtracking (an algorithm strategy). It is certainly more efficient than recursion.acdcjunior
I agree that backtracking is the way to go. It should be fairly straight forward with a Stack implementation. .3xil3
Code looks good. Try to print entire board at each step and check if it behaves as it should.Piotr Jaszkowski

1 Answers

1
votes

So your problem lies here:

previous.fillRemainderOfBoard(this);

and here

next.fillRemainderOfBoard(this);

Basically you have too many function calls on the stack because you are going forward and backward multiple times. You are not really returning from any call before answer is found (or you get StackOverflowError :)). Instead of increasing stack size by using recursive function try to think how to solve problem using a loop (pretty much always loop is better than recursive solution performance wise). Ok so you can do something like this (pseudo code):

boolean fillRemainderOfBoard(Square sender){
 if (num_defined_on_start) {
     return next.fillRemainderOfBoard(this);
 } else {
   for (i = 1; i < 9; ++i) {
     if (setting i a legal_move)
        this.setCurrentPos(i);
     else
        continue;
     if (next.fillRemainderOfBoard(this))
       return true;
   }
   return false;
}

The stack size will be ~81.