Return to Answer

The return in is_magic_square could probably be broken up too. I think it's fine, but it might make some people gag. It could also cleaned up a bit using all too:

The return in is_magic_square could probably be broken up too. I think it's fine, but it might make some people gag. It could be cleaned up a bit using all:

• Checks that each sums correctly:

  • Rows
  • Columns
  • Diagonals

• Check that the square is in fact a square.

• Check that it contains the correct set of numbers.

• Check that each sums correctly:

  • Rows
  • Columns
  • Diagonals

• Check that the square is in fact a square.

• Check that it contains the correct set of numbers.

I normally don't like to do complete rewrites for reviews as I don't think that they're usually helpful. Here though, the major problem that I see with your code is you're trying to do far much "manually". You aren't making good use of built-in Python constructs that automate some of the painful elements. You also have everything in one massive block. I rewrote this from scratch to show how I'd approach the problem fresh.

I see each of these as distinct problems that should be handled separately. In your current code, you have everything mixed together in one massive function which makes it difficult to tell what is responsible for what job. It simply is not easy code to read.

The return in is_magic_square could probably be broken up too. I think it's fine, but it might make some people gag.

I normally don't like to do complete rewrites for reviews as I don't think that they're usually helpful. Here though, the major problem that I see with your code is you're trying to do far too much "manually". You aren't making good use of built-in Python constructs that automate some of the painful elements. You also have everything in one massive block. I rewrote this from scratch to show how I'd approach the problem fresh.

I see each of these as distinct problems that should be handled separately. In your current code, you have everything mixed together in one massive function which makes it difficult to tell what is responsible for what job. It's simply not very easy code to read.

The return in is_magic_square could probably be broken up too. I think it's fine, but it might make some people gag. It could also cleaned up a bit using all too:

return all([is_normal_square(grid),
            has_correct_dimensions(grid),
            check(grid),
            check(zip(*grid)),
            check([diagonal_of(grid, lambda x: x),
                   diagonal_of(grid, lambda x: len(grid) - x - 1)])])

At least that gets rid of the ugly line continuations.