-
-
Notifications
You must be signed in to change notification settings - Fork 21
London | July SDC | Ali Qassab | Sprint 4 | Implement laptop allocation #46
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?
Conversation
LonMcGregor
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.
Good start on this task - I've left some comments for places where you could think more about the efficiency of the implementation.
prep-exercises/laptop_allocation.py
Outdated
| sadness = calculate_sadness(person, laptop) | ||
| print(f"{person.name} -> {laptop.manufacturer} {laptop.model} ({laptop.operating_system.value}) - Sadness: {sadness}") | ||
|
|
||
| print(f"\nTotal Sadness: {calculate_total_sadness(allocation)}") |
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.
Thinking about efficiency again, you just looped over everyone to get their individual sadness. The function you're calling here uses a loop as well. Is there any other way of achieving this without needing to loop twice here?
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.
Yes, with these changes:
- Add total_sadness = 0 before the loop
- Accumulate total_sadness += sadness inside the loop
- Replace the calculate_total_sadness(allocation) call with total_sadness
prep-exercises/laptop_allocation.py
Outdated
| # Create a priority queue of (person, laptop, sadness) tuples | ||
| # Sort by sadness to allocate best matches first | ||
| preferences = [] | ||
| for person in people: |
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.
In this approach you are looping over all people and laptops,then afterwards sorting and then doing the assignement.
Is there a way you could achieve this with fewer loops, or not going fully around the loops you have so much?
Imagine if you were doing this for a class full of hundreds of laptops and people - how many times would you end up going around the loops?
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.
For 100 people and 100 laptops:
Before: 10,000 pairs created + sorted
After: At most 10,000 sadness calculations, but typically much fewer due to early exits and shrinking sets
LonMcGregor
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.
Perfect!
Learners, PR Template
Self checklist
Changelist
Added solution to allocation laptop and sadness
Questions
Ready to review