java magic square out of bounds exception

Question

I have written code for a magic square generator that takes an odd int(n). I used the standard rows = n-1 and columns = n/2. I am not sure why my code has an out of bounds exception. If anyone with a fresh pair of eyes could please point out my error it would be much appreciated.

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Scanner;

public class part1_2 {

    public static void main(String[] args) {
        System.out.println("input an odd value of n");
        Scanner ns = new Scanner(System.in);
        int n = ns.nextInt();
        int ROWS = n;
        int COLUMNS = n;
        int[][] values = new int[ROWS][COLUMNS];
        while (n % 2 == 0) {
            System.out.println("n must be odd");
            n = ns.nextInt();
        }
        int[] list = new int[n*n ];
        for(int x = 1; x < (n*n+1); x++) {
            list[x-1]=x;
        }
        int counter = 0;
        for(int r = (n-1); r< n+1; r ++) {
            for(int c = (n/2); c<(n+1); c++) {
                values[r][c] = list[counter];
                if(c+1 == n) {
                    c = 0;
                }
                counter++;
                //}
                //list.remove(counter);
                //counter++;
                //System.out.println();
                //}
                //System.out.println(values[1][1]);
            }
            if(r+1 == n) {
                r = 0;
            }
        }
        System.out.println(values);
    }
}

I have formatted your code for readability. In the future, you should be doing this — Hovercraft Full Of Eels, Jan 02 '18 at 13:10
Rather than accepting input, please just hard-code the value that's giving you the incorrect result. Then include the stack trace in the question - at the moment we've no idea where it's failing... or what you've tried in order to diagnose the problem. — Jon Skeet, Jan 02 '18 at 13:10
The double `for` loops look fishy to me, but I don't see an obvious problem. You could try debugging your code with small values of `n` (say `n=3`) and try to catch the problem happening. Anyway, even if someone posts an answer here you would likely have to test it yourself. — Tim Biegeleisen, Jan 02 '18 at 13:11
You define `values = new int[n][n]; (rows and columns = n)` and yet later inside your loop try to set `values[r][c]` with both `r` and `c` goind up until `< n+1` is reached. So for example if `n=5` you will try to access `values[5][5]` inside the loop (because 5 < 5+1) which doesn't exist because `new int[5][5]` will create a 2 dimensional array with indexes from 0 to 4. — OH GOD SPIDERS, Jan 02 '18 at 13:15
@OHGODSPIDERS is right. But there's another problem. `int[][] values = new int[ROWS][COLUMNS];` is done before your check. So user can change the value of n after the initialization of the array `values`. It may crash again... — Genu, Jan 02 '18 at 13:34

score 0 · Accepted Answer · answered Jan 02 '18 at 13:33

        if(c+1 == n) {
            c = 0;
        }
        if(r+1 == n) {
            r = 0;
        }

The problem is, that you are always resetting the values of c and r to 0. The for loop handles that for you. Just say, that the for loops only go till c < n. This is the fixed part:

    int counter = 0;
    for(int r = (n-1); r< n; r ++) {
        for(int c = (n/2); c<n; c++) {
            values[r][c] = list[counter];
            counter++;
        }
    }

java magic square out of bounds exception

1 Answers1