Skip to main content
added 314 characters in body
Source Link
Roland Illig
  • 21.9k
  • 2
  • 36
  • 84

Most of your code looks good. I have a few remarks, though.

The name of your function rotate_m1 is misleading. What does the m1 mean at all? Additionally, your function prints the result. It should not do that. Instead, it should either rotate the matrix in-place, or alternatively return the rotated matrix. That way, you can use the result further:

print(rotate(rotate(matrix)))

With your current code, thisis not possible.

The docstring of your function is very long. This docstring should be written for a specific target audience, which is the users of the function. The users only need to know:

"""Returns a copy of the given matrix, rotated clockwise by 90 degrees.

Example: [[1, 2, 3], [4, 5, 6]] is rotated to [[4, 1], [5, 2], [6, 3]]
"""

The implementation details do not belong in the behavioral description of the function.

Note that I chose a non-square example on purpose. If you rotate the matrix in-place, the matrix has to be square, but when you create a copy, you can also handle arbitrary rectangular matrices. Your code currently doesn't do this, but it should.

My standard disclaimer about CtCI: Don't trust that book. It contains many typos and inefficient, English-America-centric code. The "solution" code is often not indented properly, variables are not named carefully, edge cases are forgotten, and so on. I would not hire someone who learned from that book alone.

Most of your code looks good. I have a few remarks, though.

The name of your function rotate_m1 is misleading. What does the m1 mean at all? Additionally, your function prints the result. It should not do that. Instead, it should either rotate the matrix in-place, or alternatively return the rotated matrix. That way, you can use the result further:

print(rotate(rotate(matrix)))

With your current code, thisis not possible.

The docstring of your function is very long. This docstring should be written for a specific target audience, which is the users of the function. The users only need to know:

"""Returns a copy of the given matrix, rotated clockwise by 90 degrees.

Example: [[1, 2, 3], [4, 5, 6]] is rotated to [[4, 1], [5, 2], [6, 3]]
"""

The implementation details do not belong in the behavioral description of the function.

Note that I chose a non-square example on purpose. If you rotate the matrix in-place, the matrix has to be square, but when you create a copy, you can also handle arbitrary rectangular matrices. Your code currently doesn't do this, but it should.

Most of your code looks good. I have a few remarks, though.

The name of your function rotate_m1 is misleading. What does the m1 mean at all? Additionally, your function prints the result. It should not do that. Instead, it should either rotate the matrix in-place, or alternatively return the rotated matrix. That way, you can use the result further:

print(rotate(rotate(matrix)))

With your current code, thisis not possible.

The docstring of your function is very long. This docstring should be written for a specific target audience, which is the users of the function. The users only need to know:

"""Returns a copy of the given matrix, rotated clockwise by 90 degrees.

Example: [[1, 2, 3], [4, 5, 6]] is rotated to [[4, 1], [5, 2], [6, 3]]
"""

The implementation details do not belong in the behavioral description of the function.

Note that I chose a non-square example on purpose. If you rotate the matrix in-place, the matrix has to be square, but when you create a copy, you can also handle arbitrary rectangular matrices. Your code currently doesn't do this, but it should.

My standard disclaimer about CtCI: Don't trust that book. It contains many typos and inefficient, English-America-centric code. The "solution" code is often not indented properly, variables are not named carefully, edge cases are forgotten, and so on. I would not hire someone who learned from that book alone.

Source Link
Roland Illig
  • 21.9k
  • 2
  • 36
  • 84

Most of your code looks good. I have a few remarks, though.

The name of your function rotate_m1 is misleading. What does the m1 mean at all? Additionally, your function prints the result. It should not do that. Instead, it should either rotate the matrix in-place, or alternatively return the rotated matrix. That way, you can use the result further:

print(rotate(rotate(matrix)))

With your current code, thisis not possible.

The docstring of your function is very long. This docstring should be written for a specific target audience, which is the users of the function. The users only need to know:

"""Returns a copy of the given matrix, rotated clockwise by 90 degrees.

Example: [[1, 2, 3], [4, 5, 6]] is rotated to [[4, 1], [5, 2], [6, 3]]
"""

The implementation details do not belong in the behavioral description of the function.

Note that I chose a non-square example on purpose. If you rotate the matrix in-place, the matrix has to be square, but when you create a copy, you can also handle arbitrary rectangular matrices. Your code currently doesn't do this, but it should.