Glasgow | 25-SDC-NOV | Katarzyna Kazimierczuk | Sprint 5 | laptop task function#328
Glasgow | 25-SDC-NOV | Katarzyna Kazimierczuk | Sprint 5 | laptop task function#328katarzynakaz wants to merge 1 commit intoCodeYourFuture:mainfrom
Conversation
OracPrime
left a comment
There was a problem hiding this comment.
It could do with some error checking on the OS choices - although most people just used test data rather than asking for user input, so I'd suggest make a note of this for future use, but don't bother adding it here.
I'd also suggest a loop rather than the duplicated code. However I'm sure you understand the suggestion, so there's no need at this stage to fix it (given how much of a review backlog we volunteers have built up!)
So while there is room for improvement, I think it's close enough to approve.
| sys.exit(1) | ||
|
|
||
| add_age = int(input("What is your age? \n")) | ||
| if add_age < 0 or add_age > 100: |
There was a problem hiding this comment.
Pessimistic view of life expectancy :(
| def create_a_new_person() -> Person: | ||
| add_name = input("What is your name? \n").title() | ||
| if not add_name or add_name == "" or len(add_name) > 50: | ||
| print("Enter a valid name.", file=sys.stderr) |
There was a problem hiding this comment.
I'm Liking the stderr for error messages!
| # user_oss_ranked_as_enums = [choice1, choice2, choice3] | ||
| # or shorter | ||
| #here i have strings converted to enums so can compare | ||
| user_oss_ranked_as_enums = [ |
There was a problem hiding this comment.
What happens if what they enter doesn't match any of the values?
| if laptop.operating_system == person.preferred_operating_system[0]: | ||
| return laptop, 0 | ||
|
|
||
| for laptop in laptops: |
There was a problem hiding this comment.
It's a bit odd having this written out 3 times rather than a loop
Self checklist
Changelist
This is the laptop allocation task with the logic changed from the previous work.