magic square wrong placement of some numbers

Question

i tried to create a simple 3x3 magic square. 3x3 magic square consists of consecutive integers (starting with 1 and ending with 9) placed into 'n' rows by 'n' columns so that all rows, all columns and both diagonals sum to the same total.

My algorithm is 1 up, 1 left.

my problem is that i am having difficulties figuring out why i cant retain my previous number and make go down 1 if the next column is occupied by a number. thank you in advance

#include <iostream>
using namespace std;

int main ()
{
int magicsq[3][3];
int i,j,x;

int row=0;      //start positon of row
int col=3/2;    // and colum

    for( i=0;i<3;i++)
    {
        for(j=0;j<3;j++)
        {
            magicsq[i][j] = 0;      //initialize to 0 your matrix
        }   
    }

 magicsq[row][col] = 1;     //position to star the counting

 for(x=2;x<3*3;x++)
 {
    row--;
    col--;

    if(row<0){
        row=row+3;
     }

    if(col<0){
        col=col+3;
    }
    if(magicsq[row][col]!=0)
    {

        row++;                  //i think this is where im having trouble

    }

    magicsq[row][col] = x;
 }

 for( i = 0; i<3;i++){
    for(j = 0; j<3;j++)
    {
        cout<<magicsq[i][j] <<" ";
     }
     cout<<endl;
 }
}

Answer 1

Division by integer issue; that's all. 3/2 is 1.

if(magicsq[row][col!=0]) is also valid (the column selected will be either 0 or 1), but I think you mean if(!magicsq[row][col])

Answer 2

I head never heard about Magic Squares before and consulted Wikipedia about them. As expected, I found the description of various algorithms for their construction.

I identified the algorithm of de la Loubère as the one which the OP obviously tried to implement. IMHO, the attempt was actually not that bad.

The hints I gave about (missing row wrap-around in) if(magicsq[row][col]!=0) row++; and (missing iteration step in) for (x=2;x<3*3;x++) appeared to be reasonable.

Considering this I got the algorithm running. My first result looked not that bad but the result checks showed that it was actually wrong. I switched some directions of row and column counting but without luck. Then, I re-visited the above linked article again and compared what was described with what I had implemented. Finally, I found out that I made another semantical mistake in my implementation. I believe the OP did it as well:

The method prescribes starting in the central column of the first row with the number 1. After that, the fundamental movement for filling the squares is diagonally up and right, one step at a time. If a filled square is encountered, one moves vertically down one square instead, then continues as before. When an "up and to the right" move would leave the square, it is wrapped around to the last row or first column, respectively.

Emphasize of instead was done by me – the detail I missed. So, there was a change necessary: The coordinates of new cell have to be discarded if the cell is already filled. The new coordinates are instead one row down (i.e. ++row) with the resp. wrap-around. After fixing this the sample computed a correct result:

#include <iostream>

int main()
{
  int magicsq[3][3] = {0}; // initializes magic square with all 0s
  // Algorithm of de la Loubere:
  int row = 0, col = 1; // start coordinates
  // assign start cell
  magicsq[row][col] = 1;
  for (int x = 2; x <= 9; ++x) {
    // compute up-right coordinates
    int rowT = row - 1; if (rowT < 0) rowT += 3;
    int colT = col + 1; if (colT >= 3) colT -= 3;
    // check whether cell not yet assigned
    if (magicsq[rowT][colT]) {
      // compute down coordinates
      if (++row >= 3) row -= 3;
    } else {
      // use up-right coordinates
      row = rowT; col = colT;
    }
    // assign next cell
    magicsq[row][col] = x;
  }
  // output of result:
  std::cout << "Magic Square:" << std::endl;
  for (row = 0; row < 3; ++row) {
    for (col = 0; col < 3; ++col) {
      std::cout << ' ' << magicsq[row][col];
    }
    std::cout << std::endl;
  }
  // check result:
  std::cout << "Row sums:";
  for (row = 0; row < 3; ++row) {
    int sum = 0;
    for (col = 0; col < 3; ++col) sum += magicsq[row][col];
    std::cout << ' ' << sum;
  }
  std::cout << std::endl;
  std::cout << "Column sums:";
  for (col = 0; col < 3; ++col) {
    int sum = 0;
    for (row = 0; row < 3; ++row) sum += magicsq[row][col];
    std::cout << ' ' << sum;
  }
  std::cout << std::endl;
  std::cout << "Diagonal sums: ";
  int sumM = 0, sumC = 0;
  for (row = 0; row < 3; ++row) {
    sumM += magicsq[row][row];
    sumC += magicsq[row][2 - row];
  }
  std::cout << ' ' << sumM << ' ' << sumC << std::endl;
  // done
  return 0;
}

The output is:

Magic Square:
 8 1 6
 3 5 7
 4 9 2
Row sums: 15 15 15
Column sums: 15 15 15
Diagonal sums:  15 15

Life demo on ideone

Note:

Implementing such an algorithm from description, you can easily be trapped. The description used "row up", "row down" according to the illustrations. So, a clear mapping is needed to define what "up" and "down" means concerning the matrix storage. In my case, "row up" is row decrement, "row down" is row increment. In this specific case, it probably works also vice versa as the linked article mentions that result matrix can be mirrored horizontally and vertically.