Matrix rotation algorithm - follow-up

Question

In a response to this post I wrote a solution in Python. It passed all the test cases given, so I would like help making it more pythonic and reducing the almost repeated lines.

Link to the source of the question on HackerRank.

# input
M, N, rotations = map(int, input().split())
matrix = []
for row in range(M):
 matrix.append(list(map(int, input().split())))
# flatten the rings into a list
layers = int(min(N, M)/2)
ring = [[] for layer in range(layers)]
for level in range(layers):
 top = (N-1) - 2 * level
 side = (M-1) - 2 * level
 for row in range(top): # right
 ring[level].append(matrix[level][level + row])
 for col in range(side): # down
 ring[level].append(matrix[level + col][level + top])
 for row in range(top): # left
 ring[level].append(matrix[level + side][level + top - row])
 for col in range(side): # up
 ring[level].append(matrix[level + side - col][level]) 
# rotate each layer
for level in range(layers):
 r = rotations % len(ring[level])
 ring[level] = ring[level][r:] + ring[level][:r]
# fill the array with the rotated elements
for level in range(layers):
 top = (N-1) - 2 * level
 side = (M-1) - 2 * level
 for row in range(top):
 matrix[level][level + row] = ring[level].pop(0) # right
 for col in range(side):
 matrix[level + col][level + top] = ring[level].pop(0) # down
 for row in range(top):
 matrix[level + side][level + top - row] = ring[level].pop(0) # left
 for col in range(side):
 matrix[level + side - col][level] = ring[level].pop(0) # up
# print the rotated matrix
for row in range(M):
 for col in range(N):
 print(matrix[row][col], "", end="")
 print()

• holroy: Don't forget to accept one of the answers, or comment on why you don't find any of the current answers good enough for acception.

Answer 1

List comprehensions

Whenever you see a list initialization to [], later filled in by .append(...) calls in a loop, consider rewriting with a list comprehension. For example:

matrix = []
for row in range(M):
 matrix.append(list(map(int, input().split())))

Can be rewritten on a single line:

matrix = [list(map(int, input().split())) for _ in range(M)]

Note that I renamed row to _, this is a common convention when then loop variable is unused.

Another example of that is this list comprehension later in your code:

ring = [[] for layer in range(layers)]

Like earlier, rename layer to _:

ring = [[] for _ in range(layers)]

Integer division with //

Instead of this:

layers = int(min(N, M)/2)

Another, a more natural way to achieve the same effect using integer division with //:

layers = min(N, M) // 2

Coding style

The body of this for-loop is overindented:

# rotate each layer
for level in range(layers):
 r = rotations % len(ring[level])
 ring[level] = ring[level][r:] + ring[level][:r]

Possibly it's a result of the copy-pasting? Either way, a PEP8 checker would catch this. It's recommended to use four spaces for an indent level, and to use only spaces and never tabs for indenting code.

Reducing duplicated code

The current implementation works like this:

1. put each layer into ring
2. rotate the values in each layer in ring
3. overwrite the layers in the matrix from ring

Step 1 and 3 have very similar code, as you yourself noticed. One way to reduce that is to implement shifting in-place, without using the additional storage ring, by doing something like this (\$k\$ is the number of rotations):

1. save the first \$k\$ items in a list
2. overwrite the \$i\$-th values by the \$(i+k)\$-th values. The last \$k\$ values will come from the values we saved in the previous step.

Easier said than done? Yes. Calculating the \$(i + k)\$-th position will be tricky.

I'd say your original implementation with the similar lines is fine: it's easy to understand, and it passed the tests anyway.