Skip to content

Juwei95 A3#46

Open
juwei95 wants to merge 8 commits into
mainfrom
juwei95-A3
Open

Juwei95 A3#46
juwei95 wants to merge 8 commits into
mainfrom
juwei95-A3

Conversation

@juwei95

@juwei95 juwei95 commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

Ready for review :)

@kralina123 kralina123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very thoughtfully implemented solution! I like the additional Bellman-Ford option. Your parsing is very clean and robust. I also like that you defined a Node dataclass, as it neatly stores the score and the traceback information. The README is very nicely structured and provides clear instructions. I also appreciate that you created an own test file for the tests.

However, I noticed some potential improvements:

The assignment specifies that input files may contain decimal numbers with commas. In the current implementation, values are parsed using 'float(weight)'. This will fail for inputs like 1,25. It would be better to replace commas with dots before conversion, e.g. 'float(weight.replace(",", "."))'.

According to the assignment, moving south should be preferred in case of equal values. In this implementation, updates are only performed when 'new_score > next_node.score'. However, you start with moving right 'for edges in (right, down, diagonal)', so in case of equal scores it would move east.

Overall, very nicely done and some good approaches!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants