# Test whether or not a 4x4 matrix is a magic square

Here is what I have. I have it outputting most of the sums just so I can check there values. I think the problem is with the value of the elements in the array storing the column sums. I would greatly appreciate any feedback.

#include <iostream>
#include <cmath>
using namespace std;

void fillMatrix(int matrix[][4],const int SIZE);
int rowSum(int matrix[][4],const int SIZE,int row[]);
int columnSum(int matrix[][4], const int SIZE, int column[]);
bool isMagic(int matrix[][4], const int SIZE,int row[],int column[]);

int main()
{
const int SIZE=4;
int matrix[SIZE][SIZE];
int row[4],column[4];//arrays to be filled with row and column sums.
char response=0;

cout<<"This program determines whether or not a 4x4 square matrix is a magic square.\n";
do
{
fillMatrix(matrix,SIZE);
rowSum(matrix,SIZE,row);
columnSum(matrix,SIZE,row);

if(isMagic(matrix,SIZE,row,column))
cout<<"This is a magic square.\n\n";
else {
cout<<"This is not a magic square.\n\n";
}
cout<<"To end this program, enter q. To check another matrix, enter any other letter.\n";
cin>>response;
}while(response!='q'&&response!='Q');

return 0;
}

void fillMatrix(int matrix[][4],const int SIZE)
{
for(int i=0;i<4;i++)
{
cout<<"Enter four values for row "<<i+1<<".\n";
for(int j=0;j<4;j++)
{
cin>>matrix[i][j];
}
}
}

int rowSum(int matrix[][4],const int SIZE,int row[4])
{
int i=0;
int rowsum=0;
for(i=0;i<4;i++)
{
rowsum=0;
for(int j=0;j<4;j++)
{
rowsum+=matrix[i][j];
}
row[i]=rowsum;
cout<<row[i]<<endl;
}
return row[i];

}
int columnSum(int matrix[][4], const int SIZE, int column[4])
{
int j=0;
int columnsum=0;
for(j=0;j<4;j++)
{
columnsum=0;
for(int i=0;i<4;i++)
{
columnsum+=matrix[i][j];
}
column[j]=columnsum;
cout<<column[j]<<endl;
}
return column[j];
}

bool isMagic(int matrix[][4], const int SIZE,int row[4],int column[4])
{
int rightdiagonalsum=0, leftdiagonalsum=0, check;

for(int i=0;i<4;i++)
{
rightdiagonalsum+=matrix[i][i];
}
cout<<rightdiagonalsum<<endl;
for(int i=0;i<4;i++)
{
leftdiagonalsum+=matrix[i][3-i];
}
cout<<leftdiagonalsum<<endl;
for(int i=1;i<4;i++)
{
if (row[i]==row[i-1])
{
check=row[i];
}
else {
return false;
}
}
for(int j=0;j<4;j++)
{
if (column[j]!=check)
{
cout<<column[j]<<"*****";//For some reason, the value of column[j] is 0.
return false;
}
}

if (rightdiagonalsum!=check||leftdiagonalsum!=check)
{
return false;
}

return true;
}

• You seem to lack the outermost level of indentation. If that is an artifact of pasting the code into Code Review, please be aware that the Stack Exchange text editor can indent a block of code if you click the "{}" button. Commented Feb 23, 2014 at 17:22

The size of the matrix is hard-coded all over the place. You made a good attempt to define const int SIZE = 4 in main(), and you pass SIZE to your functions. However, you still hard-code int matrix[][4], int row[4], and int column[4] in the function signatures. In addition, fillMatrix() says "Enter four values for row ".
• The simplest is to #define SIZE 4, and don't bother passing the size around anymore. It's more of a C-style approach, but at least the size will be centrally defined.
• You could store the matrix in a std::array<std::array<int>> (C++11), a std::vector<std::vector<int>>, or a std::valarray<std::valarray<int>>. Then the helper functions could obtained the dimensions by calling .size().
• You could define a Matrix class.