StackOverflowError in Magic Square program for checking if square is magic

Question

I guess I'll get to the point: my teacher for computer science gave us an assignment where he wanted us to create a program that generates a 3 by 3 magic square (meaning that all rows, columns and diagonals of the square must equal 15). He wanted us to use a regular array (an array with one dimension, not two) and have at least two functions - one that is recursive and that generates or shuffles the square and another that just checks if the square is magic. The program should return and print a magic square and no user input is required.

Here is my code (I'm putting this first since the problem I'm having feels easier to explain if it's placed first; skip to the end for the actual questions):

public class MagicSquare {
    public static void main(String[] args) {
        // main stub, get user input here
        int[] square = {1, 2, 3, 4, 5, 6, 7, 8, 9};
        //int[] test = {2, 7, 6, 9, 5, 1, 4, 3, 8};

        //printMagicSquare(test);

        shuffleSquare(square, 0);       
        printMagicSquare(square);
    }

    public static int[] shuffleSquare(int[] square, int count) {
        // shuffles array
        Random randGen = new Random();

        if(count >= square.length-1) {
            return square;
        }
        else {
            int index = randGen.nextInt(square.length - 1) + 0;
            int temp = square[count];
            square[count] = square[index];
            square[index] = temp;

            shuffleSquare(square, count + 1);
        }
        return square;
    }

    public static boolean checkIfMagic(int[] square) {
        // returns true or false for whether or not inputted array is a magic square
        int MAGICNUM = 15;

        int row1 = square[0] + square[1] + square[2];
        //System.out.println(square[0] + " " + square[1] + " " + square[2]);
        int row2 = square[3] + square[4] + square[5];
        //System.out.println(square[3] + " " + square[4] + " " + square[5]);
        int row3 = square[6] + square[7] + square[8];
        //System.out.println(square[6] + " " + square[7] + " " + square[8] + "\n");

        int col1 = square[0] + square[3] + square[6];
        int col2 = square[1] + square[4] + square[7];
        int col3 = square[2] + square[5] + square[8];

        int diag1 = square[0] + square[4] + square[8];
        int diag2 = square[2] + square[4] + square[6];

        if(row1 == MAGICNUM && row2 == MAGICNUM && row3 == MAGICNUM && col1 == MAGICNUM && col2 == MAGICNUM && col3 == MAGICNUM && diag1 == MAGICNUM && diag2 == MAGICNUM) {
            return true;
        }
        else {
            return false;
        }
    }

    public static void printMagicSquare(int[] square) {
        // prints out magic square
        boolean isMagic = checkIfMagic(square);
        // check if square is magic (if it is, print it, if not then re-shuffle it and re-check it)
        if(isMagic == true) {
            System.out.println("Magic Square: ");
            for(int count = 0; count < square.length; count ++) {
                if(count == 3 || count == 6) {
                    System.out.println();
                    System.out.print(square[count] + " ");
                }
                else {
                    System.out.print(square[count] + " ");
                }
            }
            System.out.println("\n");
        }
        else {
            shuffleSquare(square, 0);
            printMagicSquare(square);
        }
    }
}

So, the problem I'm having is that the program stops shuffling the square after a certain amount of times. The functions shuffleSquare and checkIfMagic both work, it just gives a stackOverflowError after re-shuffling n times. I tested to see if it would do this if I removed some restrictions in the checkIfMagic function (for example, I tried if(row1 == MAGICNUM && row2 == MAGICNUM && row3 == MAGICNUM)) and it didn't. Instead, it outputted what exactly what it should have: a square where the rows' sums were equal to 15. It started showing the stackOverflowError when the code was if(row1 == MAGICNUM && row2 == MAGICNUM && row3 == MAGICNUM && col1 == MAGICNUM && col2 == MAGICNUM && col3 == MAGICNUM). However, since the square must be magic (again, having the sum of all rows, columns and diagonals equal the same value), I cannot use this.

I guess my main questions are how do I fix this error so that it instead continues to shuffle until it returns a magic square and if there is a better place or way to re-shuffle the square until it is magic.

You need to add some println's into your code to see that your recursive stopping condition is never achieved and why. Your recursion is somewhat complex in you have methods calling other methods that then call themselves and the first method. — Hovercraft Full Of Eels, Dec 17 '18 at 04:26
@HovercraftFullOfEels thank you for the tips! I shall keep that in mind the next time I post a question. — awkward-dingus, Dec 20 '18 at 01:30

W.H · Answer 1 · 2018-12-18T16:10:29.363

try this:

public class MagicSquare {
    public static void main(String[] args) {
        // main stub, get user input here
        int[] square = {1, 2, 3, 4, 5, 6, 7, 8, 9};
        printMagicSquare(square);
    }
    static Random randGen = new Random();
    public static void shuffleSquare(int[] square, int i) {
        if (i > 0) {
            int index = randGen.nextInt(i);
            int temp = square[index];
            square[index] = square[i];
            square[i] = temp;
            shuffleSquare(square, i - 1);
        }
    }

    public static boolean checkIfMagic(int[] square) {
        // returns true or false for whether or not inputted array is a magic square
        int MAGICNUM = 15;

        int row1 = square[0] + square[1] + square[2];
        //System.out.println(square[0] + " " + square[1] + " " + square[2]);
        int row2 = square[3] + square[4] + square[5];
        //System.out.println(square[3] + " " + square[4] + " " + square[5]);
        int row3 = square[6] + square[7] + square[8];
        //System.out.println(square[6] + " " + square[7] + " " + square[8] + "\n");

        int col1 = square[0] + square[3] + square[6];
        int col2 = square[1] + square[4] + square[7];
        int col3 = square[2] + square[5] + square[8];

        int diag1 = square[0] + square[4] + square[8];
        int diag2 = square[2] + square[4] + square[6];

        if(row1 == MAGICNUM && row2 == MAGICNUM && row3 == MAGICNUM && col1 == MAGICNUM && col2 == MAGICNUM && col3 == MAGICNUM && diag1 == MAGICNUM && diag2 == MAGICNUM) {
            return true;
        }
        else {
            return false;
        }
    }

    public static void printMagicSquare(int[] square) {
        while (!checkIfMagic(square)){
            shuffleSquare(square, square.length - 1);
        }

        System.out.println("Magic Square: ");
        for(int count = 0; count < square.length; count ++) {
            if(count == 3 || count == 6) {
                System.out.println();
                System.out.print(square[count] + " ");
            }
            else {
                System.out.print(square[count] + " ");
            }
        }
        System.out.println("\n");
    }
}

The stack memory of local variables is released only after the variables goes out scope. When you call printMagicSquare inside printMagicSquare. The caller's parameter is still in scope thus the memory is not released. The runtime creates new stack frames above the old one to store the callee's local variables and parameters. And this goes on repeatedly until one printMagicSquare finds the answer or when we running of out stack memory. Because shuffleSquare doesn't work, there is only one way to go.

    while (!checkIfMagic(square)){
        shuffleSquare(square, square.length - 1);
    }

This one makes no recursive calls, it keeps the stack top stable even if it runs forever.

Please explain your answer rather than simply provide a code dump. Without explanation of the OP's problem and why your solution helps solve it, the answer is not helpful to future visitors. — Hovercraft Full Of Eels, Dec 17 '18 at 04:35
feel free to modify the answer or post a new answer if you think it's beyond remedy ok. I'm out for dinner. — W.H, Dec 17 '18 at 04:54
@JohnSmith thank you for your help! I went to my teacher today to ask if this code was okay; he said I misunderstood what he meant and helped me understand what he wanted. I had to redo some of the code and managed to solve it. Sorry for the trouble and thank you again! — awkward-dingus, Dec 20 '18 at 01:29

StackOverflowError in Magic Square program for checking if square is magic

1 Answers1