Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/tdd_workshop/cli.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import click
from tdd_workshop.io import read_coordinates # type: ignore
from tdd_workshop.io import read_coordinates, treasure_hunt # type: ignore


@click.command()
@click.argument("file_name", type=click.Path(exists=True))
def main(file_name: str) -> None:
coordinates = read_coordinates(file_name)
print(f"Read file: {file_name}, coordinates: {coordinates}")
treasure_location = treasure_hunt(coordinates)
print(f"Found the treasure at: {treasure_location}")
15 changes: 13 additions & 2 deletions src/tdd_workshop/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from typing import Union, List, Tuple


def read_coordinates(file_name: Union[str, Path]) -> List[Tuple[str, str]]:
def read_coordinates(file_name: Union[str, Path]) -> List[Tuple[float, float]]:
"""
Will read a file of comma separated strings and turn it
into a list
Expand All @@ -15,5 +15,16 @@ def read_coordinates(file_name: Union[str, Path]) -> List[Tuple[str, str]]:
with open(file_name, "r", encoding="utf-8") as input_file:
for line in input_file:
x, y = line.strip().split(",")
coordinates.append((x, y))
coordinates.append((float(x), float(y)))
return coordinates


def calculate_slope(x1, x2, y1, y2):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One option is to just keep the tuples, that might make it more readable.

return (y2 - y1) / (x2 - x1)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add a docstring, as it is not immediately apparent. Also the name is not very descriptive.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your style check is failing, one reason is because we are missing type hints in this function.

for i, (x2, y2) in enumerate(input_coordinates[1:]):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is a bit confusing, perhaps it is worth rewriting?

x1, y1 = input_coordinates[i]
if calculate_slope(x1, x2, y1, y2) > 1:
return x2, y2
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return x2, y2
return (x2, y2)

9 changes: 3 additions & 6 deletions tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@ def test_that_cli_fails_when_file_is_missing():

def test_that_cli_prints_coordinates(monkeypatch, tmp_path):
monkeypatch.chdir(tmp_path)
expected_coordinates_list = [
coordinates_list = [
("2.0", "1.9"),
("2.5", "1.5"),
("3.3", "7.3"),
]
with open("test_input", "w", encoding="utf-8") as fout:
fout.write("\n".join(",".join(coords) for coords in expected_coordinates_list))
fout.write("\n".join(",".join(coords) for coords in coordinates_list))
runner = CliRunner()
result = runner.invoke(main, ["test_input"])
assert result.exit_code == 0
assert (
"coordinates: [('2.0', '1.9'), ('2.5', '1.5'), ('3.3', '7.3')]" in result.output
)
assert "Found the treasure at: (3.3, 7.3)" in result.output
34 changes: 27 additions & 7 deletions tests/unit/test_read_coordinates.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from tdd_workshop.io import read_coordinates
import pytest

from tdd_workshop.io import read_coordinates, calculate_slope, treasure_hunt


def test_that_we_can_split_file_into_coordinates(test_data_path):
Expand All @@ -9,12 +11,30 @@ def test_that_we_can_split_file_into_coordinates(test_data_path):
the file located on disk.
"""
expected_coordinates_list = [
("1", "1"),
("2.0", "1.8"),
("2.5", "2.1"),
("3", "2"),
("4.3", "7.2"),
("5.3", "6.2"),
(1, 1),
(2.0, 1.8),
(2.5, 2.1),
(3, 2),
(4.3, 7.2),
(5.3, 6.2),
]

assert read_coordinates(test_data_path / "test_input") == expected_coordinates_list


@pytest.mark.parametrize(
"input_vals, expected_result", [[[1, 2, 1, 2], 1], [[1, 2, 1, 4], 3]]
)
def test_slope_calculator(input_vals, expected_result):
assert calculate_slope(*input_vals) == expected_result


def test_slope_calculator_without_parameterize():
assert calculate_slope(*[1, 2, 1, 2]) == 1
assert calculate_slope(*[1, 2, 1, 4]) == 3
Comment on lines +32 to +34
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate of the test above, I would suggest keeping just one of the implementations.




def test_that_slopes_are_calculated_correctly():
input_list = [(1, 1), (2.0, 1.8), (2.5, 2.1), (3, 2), (4.3, 7.2), (5.3, 6.2)]
assert treasure_hunt(input_list) == (4.3, 7.2)