Skip to main content
added 191 characters in body
Source Link
bdecaf
  • 402
  • 2
  • 9

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.

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

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.
Source Link
bdecaf
  • 402
  • 2
  • 9

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