code review request#13
Conversation
|
Hi @Mayongjin1404, I think you have sent this to the wrong person. For practical 06, you need to send it to "@haydenw98" so they can review your code? The link to the mentions list is provided below for future reference: https://github.com/CP1404/Starter/wiki/Code-Review-Order#prac-7 |
|
@haydenw98 Could you please review my practical 7 code? Thanks! |
haydenw98
left a comment
haydenw98
left a comment
There was a problem hiding this comment.
Just finished reviewing your code, though it's clear you're ahead of me when it comes to python as I am unfamiliar with some of your conventions. Great work!
| limo.add_fuel(20) | ||
| print(f"Limo fuel: {limo.fuel}") | ||
| limo.drive(115) | ||
| print(limo) |
There was a problem hiding this comment.
Very tidy work, not much to critique here!
| """ | ||
|
|
||
| from prac_06.programming_language import ProgrammingLanguage | ||
|
|
There was a problem hiding this comment.
Great work, although putting inside a main() with docstrings might help with readability
| self.year = year | ||
|
|
||
| def is_dynamic(self) -> bool: | ||
| return self.typing.lower() == "dynamic" |
There was a problem hiding this comment.
Great thinking with .lower though not sure how necessary it is.
| f"worth ${guitar.cost:10,.2f}{vintage_tag}") | ||
|
|
||
|
|
||
| def _read_int(prompt: str) -> int: |
There was a problem hiding this comment.
Hadn't seen this function naming standard before (marking private by starting with an _) so had to read up on it. May be a little restrictive or unnecessary for the program as it is defined and used in the same script?
| raise ValueError | ||
| return value | ||
| except ValueError: | ||
| print("Input must be a non-negative number") |
There was a problem hiding this comment.
Great error checking! My script missed this.
| def is_vintage(self) -> bool: | ||
| return self.get_age() >= 50 | ||
|
|
||
| def __str__(self) -> str: |
| return CURRENT_YEAR - self.year | ||
|
|
||
| def is_vintage(self) -> bool: | ||
| return self.get_age() >= 50 |
There was a problem hiding this comment.
I too used 50 here, but maybe a constant or variable would make 50 feel less like a magic number
3 participants
@jc463272 Could you please review my practical 6 code? Thanks!