Magic Square final value always true

Question

For class we have to do a magic square. I got my code somewhat working but it does multiple squares and the final one always returns 0, Here is how a magic square works.

Here is how I check

  public int downDiagSum() {
int sum = 0;
for(int r = 0; r < grid.length; r++){
    for(int c = 0; c < grid.length; c++){
        sum += grid[r][c];
    }
}
return sum;



 public int upDiagSum() {
int sum = 0;
for(int r = grid.length - 1; r >= 0; r--){
    for(int c = 0; c < grid.length; c++){
        sum += grid[r][c];
    }
}
return sum;



 public int colSum(int col) {
int sum1 = 0;
for(int r = 0; r < grid[0].length; r++){
    sum1 += grid[r][col];
}
return sum1;

public int rowSum(int row) {
int sum2 = 0;
for(int r = 0; r < grid[0].length; r++){
    sum += grid[row][r];
  }
return sum2;


 public boolean isMagicSquare() {
int num = rowSum(0);
boolean isEqual = false;
if(downDiagSum() == upDiagSum()){
    //row check
    for(int r = 0; r < grid.length; r++){
        if(rowSum(r) == num){
            isEqual = true;
        }
      }
    //column check
    for(int r = 0; r < grid.length; r++){
        for(int c = grid.length - 1; c >= 0; c--){
            if(colSum(c) == num){
                isEqual = true;
            }
        }
    }
}
return isEqual;}

The code mostly works but if I have a series it always returns true for some reason. The number set below is supposed to return false but returns true

6 32 2 34 35 1
7 11 27 28 8 30
19 14 16 15 23 24
18 20 22 21 17 13
25 29 10 9 26 12
36 5 33 4 3 31

Sorry for bad formatting i'm still very new to site. Thanks, Garrows

No! Please do not make material changes to the question that invalidate answers that people have already supplied. I have rolled back your last two edits. If you have a new question, then _ask_ a new question. Don't change a question like this once it already has answers. — Dawood ibn Kareem, Feb 04 '18 at 04:06
Oh im so sorry, im just so stumped on this. My teacher barely explained 2D arrays to us so this is so new to me. — DiamondMastiff, Feb 04 '18 at 04:11

zlakad · Accepted Answer · 2018-02-04T04:47:01.047

1

In both of your loops you are checking every row and every column. And your logic says when one row or one column sum is equal to the sum of first row (and this includes the sum of the very first row!!!), your variable isTrue becomes true (forever)! So, you should reverse your logic...

Pseudo algorithm:

isTrue = true
int magicSum = sumOfFirstDiagonal
if (magicSum != sumOfSecondDiagonal) {isTrue = false; return}
for each row
    if (magicSum != sumOfRow) {isTrue = false; return}    
for each column
    if (magicSum != sumOfColumn) {isTrue = false; return}

And here is correct way to calculate sum of diagonals:

public static int diagSumTwo() {
    int sum = 0;
    for (int r = 0; r < grid.length; r++) {
        sum += grid[r][r];
    }
    return sum;
}

public static int diagSumOne() {
    int sum = 0;
    for (int r = grid.length - 1; r >= 0; r--) {
        sum += grid[r][grid.length - 1 - r];
    }
    return sum;
}

edited Feb 04 '18 at 04:47

answered Feb 04 '18 at 03:03

zlakad

1,314
1
9
16

Im sorry, my teacher recently taught us 2d arrays. So with the current code the row check only checks rows but the column check checks both? – DiamondMastiff Feb 04 '18 at 03:26
No. You must check every row and every column and both of diagonals... But, you should check is any of these sums `!=` to "magicSum" – zlakad Feb 04 '18 at 03:35
Here is a pseudo algorithm: **1** calculate the sum of first diagonal and set the `magicSum` to this value. **2** see if the sum of second diagonal `!=` to `magicSum` and set your variable `isTrue` to false **and** return immediately **3** go to your loops for rows and columns and apply similar checking like in step **2** (Before all of this, set `isEqual = true` – zlakad Feb 04 '18 at 03:43
I've checked - your code to evaluate the sum of `downDiagSum` isn't correct. I haven't read any further. – zlakad Feb 04 '18 at 04:12
I gave you the code for calculating `downDiagSum()`. You should be able to correct your `upDiagSum()` – zlakad Feb 04 '18 at 04:18
I made mistake - the code I posted earlier for `downDiagonalSum` is actually for `upDiagonalSum`. I'm sorry for confusing you - just edited. (Or we're thinking up-side-down... – zlakad Feb 04 '18 at 04:33
You were right the first time, upDiagSum has to go from lower left to upper right so it made sense to start from bottom and work up. downDiagSum starts from upper left and goes lower right so that one needs to start at 0 and increment both row and column by 1 – DiamondMastiff Feb 04 '18 at 04:40
Well here you are. (at least for diagonals). – zlakad Feb 04 '18 at 04:48
1

I finally figured it out. I completely rewrote my isMagicSquare() method, what was messing me up was the diag checks. Thank you so much for the help! – DiamondMastiff Feb 04 '18 at 04:48

score 0 · Answer 2 · answered Feb 04 '18 at 03:09

The problem is in the isMagicSquare method.

Suppose that upDiagSum() returns the same as downDiagSum(). Your code then enters the if block and comes to the first for loop. But in the very first iteration of that first for loop, you have r = 0, which means that the internal if statement is equivalent to

if (rowSum(0) == rowSum(0))

which presumably will always be true.

So isEqual is set to true on the first iteration of the first loop, and you never set it back to false. Therefore, the return value of your method will be true, no matter what happens in subsequent iterations.

You need to change the logic so that isEqual is initially true, but becomes false when one of the sums doesn't match.

Alright I changed it up, the issue now is the last in the series returns true. — DiamondMastiff, Feb 04 '18 at 04:15

Magic Square final value always true

2 Answers2