London | 25-SDC-Nov | Jesus del Moral | Sprint 5 | Implement Laptop Allocation#297
London | 25-SDC-Nov | Jesus del Moral | Sprint 5 | Implement Laptop Allocation#297delmorallopez wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
delmorallopez
added
the
Needs Review
delmorallopez
force-pushed
the
laptop-allocation
branch
from
0a7596e to
3a77f8f
Compare
OracPrime
added
the
Review in progress
OracPrime
left a comment
OracPrime
left a comment
There was a problem hiding this comment.
Couple of minor points, one bug, but generally good code, well done.
laptop-allocation.py
Outdated
| class Person: | ||
| name: str | ||
| age: int | ||
| preferred_operating_system: Tuple[OperatingSystem, ...] |
There was a problem hiding this comment.
- Why a Tuple, not a List?
- If it is a collection, it should have a pluralised name (preferred_operating_systems)
There was a problem hiding this comment.
1.- I used Tuple instead of List to signal immutability and a fixed relationship, not something that will be modified. Items should not change.
2.- I have changed the name "preferred_operating_systems"
There was a problem hiding this comment.
Agreed, also makes it hashable, which is need for a dictionary key.
laptop-allocation.py
Outdated
|
|
||
| def allocate_laptops(people: List[Person], laptops: List[Laptop]) -> Dict[Person, Laptop]: | ||
| if len(people) != len(laptops): | ||
| raise ValueError("Number of people must equal number of laptops.") |
There was a problem hiding this comment.
Why does the number of people need to equal the number of laptops?
There was a problem hiding this comment.
In this implementation, the number of people must equal the number of laptops because the algorithm assumes a one to one assignment, every person receives exactly one laptop and every laptop is assigned to exactly one person.
There was a problem hiding this comment.
I disagree. Everyone needs a laptop, but spare laptops aren't a problem.
There was a problem hiding this comment.
You’re right, the equality check is stronger than necessary.
The algorithm only requires that there are at least as many laptops as people. Extra laptops can safely remain unassigned. The current check enforces a perfect matching, but the problem only requires that every person gets one laptop.
I have modified the code to allowed to every person must get a laptop but spare laptops are allowed
| raise ValueError("Number of people must equal number of laptops.") | ||
|
|
||
| # Clone list so we can remove laptops as they are assigned | ||
| available_laptops = laptops.copy() |
OracPrime
added
Reviewed
delmorallopez
added
the
Needs Review
OracPrime
added
Complete
2 participants
Laptop allocation app
Every person should be allocated exactly one laptop.
If we define “sadness” as the number of places down in someone’s ranking the operating system the ended up with (i.e. if your preferences were [UBUNTU, ARCH, MACOS] and you were allocated a MACOS machine your sadness would be 2), we want to minimise the total sadness of all people. If we allocate someone a laptop with an operating system not in their preferred list, treat them as having a sadness of 100