London | July SDC | Ali Qassab | Sprint 4 | Implement laptop allocation#46
London | July SDC | Ali Qassab | Sprint 4 | Implement laptop allocation#46AliQassab wants to merge 2 commits into
Conversation
AliQassab
added
📅 Sprint 4
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start on this task - I've left some comments for places where you could think more about the efficiency of the implementation.
| 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.
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.
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
| # 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.
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.
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
added
Reviewed
LonMcGregor
added
Complete
|
Closing PR because the SDC run has finished. Feel free to re-open if you're still working on it. |
3 participants
Learners, PR Template
Self checklist
Changelist
Added solution to allocation laptop and sadness
Questions
Ready to review