Conversation
oyvindeide
left a comment
oyvindeide
left a comment
There was a problem hiding this comment.
Thank you for the contribution, I think it is a good piece of work, I have some comments.
| def test_slope_calculator_without_parameterize(): | ||
| assert calculate_slope(*[1, 2, 1, 2]) == 1 | ||
| assert calculate_slope(*[1, 2, 1, 4]) == 3 |
There was a problem hiding this comment.
This is a duplicate of the test above, I would suggest keeping just one of the implementations.
| return coordinates | ||
|
|
||
|
|
||
| def calculate_slope(x1, x2, y1, y2): |
There was a problem hiding this comment.
The ordering of the input arguments are a bit hard to read, perhaps we should reorder them?
Secondly, having arguments with 1 and 2 can very often lead to bugs, so perhaps we should try to avoid that also?
There was a problem hiding this comment.
One option is to just keep the tuples, that might make it more readable.
| return (y2 - y1) / (x2 - x1) | ||
|
|
||
|
|
||
| def treasure_hunt(input_coordinates): |
There was a problem hiding this comment.
Would be good to add a docstring, as it is not immediately apparent. Also the name is not very descriptive.
There was a problem hiding this comment.
Your style check is failing, one reason is because we are missing type hints in this function.
|
|
||
|
|
||
| def calculate_slope(x1, x2, y1, y2): | ||
| return (y2 - y1) / (x2 - x1) |
There was a problem hiding this comment.
There is a potential bug here if the x-coordinate does not change. Should also add a test for that.
|
|
||
|
|
||
| def treasure_hunt(input_coordinates): | ||
| for i, (x2, y2) in enumerate(input_coordinates[1:]): |
There was a problem hiding this comment.
This line is a bit confusing, perhaps it is worth rewriting?
| for i, (x2, y2) in enumerate(input_coordinates[1:]): | ||
| x1, y1 = input_coordinates[i] | ||
| if calculate_slope(x1, x2, y1, y2) > 1: | ||
| return x2, y2 |
There was a problem hiding this comment.
| return x2, y2 | |
| return (x2, y2) |
1 participant
No description provided.