HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Magic square test

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
magictestsquare

Problem

A magic square adds up the values in each vertical, horizontal, and diagonal row to the same value. In this case, that value is 34. I have changed different indices values and the program returns false. It seems to work just fine. But I'm just learning 2d arrays. Please let me know if you see something wrong, much appreciated.

public class Blank {
    public static void main(String[] args) {
    int [][] square = {

      {16, 3, 2, 13},
      {5, 10, 11, 8},
      {9, 6, 7, 12 },
      {4, 15, 14, 1}
      }; 
   System.out.println("Is magic square: " + magicSquare(square));   
 }
   private static boolean magicSquare(int[][] square){

   //calculate the sum of the first row and assign it to n
       int n = sumOfRow(square[0]);

       for (int[] row : square)
       {
          int sum = sumOfRow(row);        
          if (sum != n)
          return false;   
       }

       int sum = 0;
       for (int i = 0; i < square.length; i++)
       {
          sum += square[i][i];
       }
       if (sum != n)
          return false;

       sum = 0;
       for (int i = 0; i < square.length; i++)
       {
          sum += square[i][square.length - 1 - i];
       }
       if (sum != n)
          return false;
       return true;
    } 

   //returns the sum of the elements in the row
   private static int sumOfRow(int[] row){
      int sum = 0;
      for(int el : row){
         sum += el;
      }
      return sum;
   }
}

Solution

There are a couple of bugs in the current implementation.

Empty array (no rows)

First of all

int n = sumOfRow(square[0]);


is problematic, because it will throw an exception when the array is empty, and this wasn't tested before. You could add an early-return testing for the particular case of an empty array:

if (square.length == 0) {
    return false; // or true, depends if you consider an empty 2D array as magic
}


Is it even a square?

There is also no verification that the 2D array is in fact a square, that is to say that each row and each column have the same length. This is also problematic because the rest of the code will raise an exception in this case, instead of simply returning false. For example, consider running it with { {1, 2}, {3} }: the row test will pass but then it'll fail at testing the diagonals. A simple way to take care of it would be the following:

for (int i = 0; i < square.length; i++) {
    if (square.length != square[i].length) {
        return false;
    }
}


We know the array has at least one row, due to the previous early-return. Note that there are tricky cases: if the input is:

int [][] square = {
  {1, 2}, // or simply {}
};


There is actually one row, and 2 columns. As such, the general algorithm is to get the number of rows of the array with array.length, and then check whether each row array[i] has that same length.

The columns

The rest of code correctly tests whether rows and both diagonals have the same sum, but it doesn't consider columns. The current algorithm considers

int [][] square = {
  {1, 2, 3},
  {5, 1, 0},
  {2, 0, 4},
};


to be a magic square, when it isn't, because the sum of the columns do not match, although the sum of each row, and each each diagonal is equal to 6.

Code Snippets

int n = sumOfRow(square[0]);
if (square.length == 0) {
    return false; // or true, depends if you consider an empty 2D array as magic
}
for (int i = 0; i < square.length; i++) {
    if (square.length != square[i].length) {
        return false;
    }
}
int [][] square = {
  {1, 2}, // or simply {}
};
int [][] square = {
  {1, 2, 3},
  {5, 1, 0},
  {2, 0, 4},
};

Context

StackExchange Code Review Q#148576, answer score: 5

Revisions (0)

No revisions yet.