-
-
Notifications
You must be signed in to change notification settings - Fork 42
Manchester | 25-SDC-Nov | Geraldine Edwards | Sprint 5 | Implement laptop allocation #286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Manchester | 25-SDC-Nov | Geraldine Edwards | Sprint 5 | Implement laptop allocation #286
Conversation
… systems and extract sadness score calculation into a separate function
…ms and improve error handling in laptop allocation
…rence lists in laptop allocation
OracPrime
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of suggested tweaks. First one more important...
| # strip() whitespace before processing (no need for str type here as input always returns a string) | ||
| name = input("Please enter your first name: ").strip() | ||
| if not name.isalpha(): | ||
| raise ValueError("Name must contain only alphabetic characters.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scarlett O'Hara might be disappointed with this parsing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks David, yes as before, I have been quite lazy about the validation - I must always remember to cover edge cases! Added validation for the first name. :)
|
|
||
| if len(allocated_laptops) != len(people): | ||
| raise ValueError("Not enough laptops to allocate one to each person.") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an "OK" solution. You might want to google Kuhn-Munkres algorithm to see how it could be improved (no need to resubmit for this, this is advanced voodoo)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually came across this algorithm when I was researching how to reduce 'sadness' for everyone but in all honesty I was a bit overwhelmed with it because the article mentioned a cost matrix and it scared me off! ( I think it was just a bit much for me at that time truth be told)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was younger you. You're wiser now :)
…ces in names and embed minimum age variable into age validation for accuracy.
|
I've added Reviewed and Complete despite adding more comments, because in the context of this exercise you've done enough! |
Learners, PR Template
Self checklist
Changelist
This PR contains the logic for implementing the laptop allocation task
Questions
Grateful for any feedback or points to consider. Thanks!