London | 26-Jul-SDC | Boshra Mahmoudi| Sprint 5 | Prep Exercises - #679
London | 26-Jul-SDC | Boshra Mahmoudi| Sprint 5 | Prep Exercises#679BoshraM wants to merge 23 commits into
Conversation
BoshraM
added
📅 Sprint 5
BoshraM
added
the
Needs Review
Khantdotcom
left a comment
Khantdotcom
left a comment
There was a problem hiding this comment.
Mandatory fix:
- ValueError in sprint5/type-guided-refactorings.py
Not mandatory but code better
- Use python's built-in methods
| def format_pence_as_string(total_pence: int) -> str: | ||
| if total_pence < 100: | ||
| return f"{total_pence}p" | ||
| pounds = int(total_pence / 100) |
There was a problem hiding this comment.
For calculating round numbers after division, check out this article (python's built-in floor division) here
There was a problem hiding this comment.
@Khantdotcom Thanks for the article. I replaced my code with // to use floor division.
| laptops = [ | ||
| Laptop(id=1, manufacturer="Dell", model="XPS", screen_size_in_inches=13, operating_system="Arch Linux"), | ||
| Laptop(id=2, manufacturer="Dell", model="XPS", screen_size_in_inches=15, operating_system="Ubuntu"), | ||
| Laptop(id=3, manufacturer="Dell", model="XPS", screen_size_in_inches=15, operating_system="ubuntu"), |
There was a problem hiding this comment.
A type here. Could raise ValueError.
There was a problem hiding this comment.
Thanks @Khantdotcom, for reviewing my code. Can you explain how it can cause a ValueError, please? I assume you are pointing to "ubuntu", which is lowercase here. In this case, I don't see any error being thrown, except that it will miss the laptop with ID 3 as a possibility. I think that's also why we use enums in the next exercise to fix this issue.
There was a problem hiding this comment.
Ah, you are completely right, @BoshraM! That was a slip on my part.
I meant to type "typo" rather than "type," and you are entirely correct that it won't raise a ValueError. Standard Python will just evaluate the string comparison to False, leading to a silent logic bug where Laptop ID 3 is missed, just as you described.
| balances[name] = amount | ||
|
|
||
| def sum_balances(accounts: dict[str, int]) -> int: | ||
| total = 0 |
There was a problem hiding this comment.
Can you find out another "short" way to calculate the sum of dict.values , using built-in method, which could save you time?
There was a problem hiding this comment.
Thanks @Khantdotcom , I've changed it to sum(accounts.values()).
Khantdotcom
left a comment
Khantdotcom
left a comment
There was a problem hiding this comment.
Clean code and sound logic. Great job and keep up the good work!!
2 participants
Task code
CYF-1155