5
\$\begingroup\$

I'm still teaching myself C out of KN King's C Programming: Modern Approach. Actually pretty excited I go this problem solved in under 2 hours. I asked this over on Stack Overflow and it was recommended I post it here.

The exercise is to print a magic square. My solution works but it feels incredibly convoluted. I'm wondering if it actually is? If anyone can help me reorganise the logic in this I'd appreciate it, or am I on the right track? Bear in mind this is Chapter 8, and we've only covered basics like loops, conditional statements, single and 2d arrays etc. I'm open to all constructive feedback.

The question:

Write a program that prints an \$n \times{} n\$ magic square (a square arrangement of the numbers \$1, 2, \ldots, n^2\$ in which the sums of the rows, columns and diagonals are all the same). The user will specify the value of \$n\$.

Store the magic square in a 2D array. Start by placing the numbers 1 in the middle of row \$0\$. Place each of the remaining numbers \$2, 3, \ldots, n^2\$ by moving up one row and over one column.

Any attempt to go outside the bounds of the array should "wrap around" to the opposite side of the array. For example, instead of storing the next number in row \$-1\$, we would store it in row \$n - 1\$ (the last row). Instead of storing the next number in column \$n\$, we would store it in column \$0\$.

If a particular array element is already occupied, put the number directly below the previously stored number. If your compiler supports variable length arrays, declare the array to have \$n\$ rows and \$n\$ columns. If not, declare the array to have \$99\$ rows and \$99\$ columns.

My answer:

// KN King Chapter 8
// Programming Projects
// Exercise 17 - Magic Square

#include <stdio.h>

int main() {

    // Introductory message
    printf("This program creates a magic sqaure of a specified size.\n");
    printf("The size must be an odd number between 1 and 99.\n");

    // Get the users magic number and allocate to int n
    int n;
    printf("Enter size of magic square: ");
    scanf("%d", &n);

    // Create the array (not using VLA)
    int magic[99][99];
    int start = (n / 2); // The middle column
    int max = n * n; // The final number
    magic[0][start] = 1; // Place the number one in the middle of row 0

    // Loop to start placing numbers in the magic square
    int row;
    int col;
    int next_row;
    int next_col;
    int i;
    for (i = 2, row = 0, col = start; i < max + 1; i++) {
        if ((row - 1) < 0) { // If going up one will leave the top
            next_row = n - 1; // Go to the bottom row
        }
        else { next_row = row - 1; } // Otherwise go up one
        printf("In row: %d\n", row);

        if ((col + 1) > (n - 1)) { // If column will leave the side
            next_col = 0; // Wrap to first column
            printf("Column will leave side\n");                                   
        }                                                                         
        else { next_col = col + 1; } // Otherwise go over one                     
        printf("In col: %d\n", col);                                              

        if (magic[next_row][next_col] > 0) { // If that position is taken         
            if (row > (n - 1)) { // If going to row below leaves bottom           
                next_row = 0; // Go back to the top                               
            }                                                                     
            else {                                                                
                next_row = row + 1; // Go to the row below                        
                next_col = col; // But stay in same column                        
            }                                                                     
        }                                                                         
        row = next_row;                                                           
        col = next_col;                                                           
        printf("About to put %d in row %d, col %d\n", i, row, col);               
        magic[row][col] = i; // Put the current value in that position            
    }                                                                             

    // Now let's print the array                                                  
    int j;                                                                        
    for (i = 0; i < n; i++) {                                                     
        for (j = 0; j < n; j++) {                                                 
            printf("%4d", magic[i][j]);                                           
        }                                                                         
        printf("\n");                                                             
    }                                                                             
    return 0;                                                                     
}
\$\endgroup\$
3
  • \$\begingroup\$ Starting the for-loop at i=2 seems a bit weird to me. I would recommend starting at i=1 and putting current value at be beginning of the loop \$\endgroup\$
    – sfk
    Commented Jan 27, 2013 at 16:03
  • \$\begingroup\$ I agree with you @sfk it does seem a little weird, but how do I place the 1 in the middle of the top row to start? I guess I could start the row and col one down and to the left so it goes up to the correct spot to place the starting 1. \$\endgroup\$
    – aussie_aj
    Commented Jan 28, 2013 at 2:53
  • \$\begingroup\$ This will help you in the magic square. The only difference in this is that it doesn't asks the user to input the 9 numbers. Instead, it does itself. When you use the code, move the srand function outside of the while loop but remains in the int main() loop. \$\endgroup\$
    – user94698
    Commented Jan 14, 2016 at 6:15

4 Answers 4

1
\$\begingroup\$

I'm just using the Ith row and Jth column formula here on wiki.

$$ n \left(\left(I + J - 1 + \left\lfloor \dfrac{n}{2} \right\rfloor \right) \mod{n}\right) + \left( \left( I + 2J - 2 \right) \mod{n} \right) + 1 $$

The loop will become:

for( i = 0; i < n; i++ ) {
  for( j = 0; j < n; j++ ) {
    magic[i][j] = n * ( (i + j - 1 + floor(n / 2)) % n ) + ( (i + 2 * j - 2) % n ) + 1;
  }
}

Ofcourse, you'd need to include # include <math.h> at the beginning so that floor() may work.

Here's the working ideone link, with sample inputs. ignore the clutter by input = 99 :)

\$\endgroup\$
3
  • \$\begingroup\$ Thanks! I didn't even know it could be done this way. That certainly cleans it up a lot. I guess for the point of the way the author wanted the exercise done mine is the way it has to be. \$\endgroup\$
    – aussie_aj
    Commented Jan 28, 2013 at 2:52
  • \$\begingroup\$ I'm sorry to criticise - but this is a completely different algorithm than posed in the exercise. \$\endgroup\$
    – bdecaf
    Commented Jan 14, 2016 at 10:43
  • \$\begingroup\$ Know your language. Including <math.h> and calling floor() is an unnecessary mess. It introduces a floating point arithmetics, while simple integer division n/2 does exactly what is needed perfectly easily. The integer result of n/2 is converted to double, then passed to floor() which returns it unchanged. Then that double result forces the whole parenthese to be performed in FP till the % operator which forces a conversion back to int. And all that just for nothing. \$\endgroup\$
    – CiaPan
    Commented Mar 3, 2021 at 14:25
1
\$\begingroup\$

I'm a beginner, studying on King. I've been doing this exercise last night. I found a simpler code, here's the ideone link: http://ideone.com/wD2EpN

The solution on the answer is really lean, but it doesn't work, rows and columns start from 0. Then it should be:

for(i = 0; i < n; i++) {
       for(j = 0; j < n; j++) {
           magic[(n -1 + i)%n][(n - 1 + j)%n] = n * ((i + j - 1 + (n - 1) / 2) % n) + ((i + 2 *     j - 2) % n) + 1;
       }
 }

By the way, I guess this is what King was asking, since we had to use vectors and 'for' loops. The exercise was asking to move columns and row "by hand".

\$\endgroup\$
1
\$\begingroup\$

As the algorithm to use actually is described in the question I would like to point out a few issues.

  1. Actually this algorithm is only for odd magic squares. But as this was not part of the exercise ignore it for now.
  2. why not use n in the allocation of magic
  3. You have lots of ifs to implement the wrapping around. A neat trick to write it more compact is to use the modulo operator it would look like: next_col = (col + 1) % n
  4. Since you are processing input it is a good practice to check if the input is valid. e.g. numbers greater 99 will completely break this thing. Also negative numbers are kinda dangerous.
\$\endgroup\$
1
\$\begingroup\$

You would save lots of code if you used a % operator to wrap indices at 0 and n-1 boundaries:

    int magic[99][99];
    int max = n * n;
    int row;
    int col;

    // make sure the array is properly initialized
    for (row = 0; row < n; row++)
        for (col = 0; col < n; col++)
            magic[row][col] = 0;

    // starting position
    row = 0;
    col = n / 2;

    int i;
    for (i = 1; i <= max; i++) {
        magic[row][col] = i;

        int next_row = (row - 1 + n) % n;
        int next_col = (col + 2) % n;

        if (magic[next_row][next_col] > 0) {
            row = (row + 1) % n;
        }
        else {
            row = next_row;
            col = next_col;
        }
    }

Note +n in next_row computation – it makes the left argument to % positive in the case of row == 0, so that the result is n-1. Without that the result might be minus one. (see e.g. Modulo operation with negative numbers)

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.