Implementation Stable marriage problem in PHP #428
Conversation
june128
added
Implementation
|
I'd say we deal with #431 before we continue with the other PHP PRs. |
removed annotations changed sprintf to printf
|
@Butt4cak3 good to go |
|
|
||
| public function setPreferences(array $preferences): void | ||
| { | ||
| shuffle($preferences); |
There was a problem hiding this comment.
I'd move this outside the setPreferences method.
| } | ||
| } | ||
|
|
||
| class Man extends Person |
There was a problem hiding this comment.
A few months ago, we had a discussion on Discord about whether we'd actually need 2 subclasses for Person and we agreed that we don't. So I need to you move the subclass code up to the Person class.
|
|
||
| function stable_marriage(array $men, array $women): void | ||
| { | ||
| $smallerGroup = count($men) < count($women) ? $men : $women; |
There was a problem hiding this comment.
Both arrays are the same size by definition. You don't have to check this.
| foreach ($women as $woman) | ||
| $woman->chooseMatch(); | ||
|
|
||
| if (empty(array_filter($smallerGroup, function (Person $person) { |
There was a problem hiding this comment.
This loops over all the objects even if the result is already determined. If the first person is single, then there is no need to loop over all the others. I recommend changing this to a foreach loop with a break statement.
|
|
||
| } while (true); | ||
|
|
||
| foreach ($women as $woman) printf('%s is married to %s%s', $woman, $woman->getMatch(), PHP_EOL); |
There was a problem hiding this comment.
Move the output out of the algorithm's function.
| } | ||
| echo PHP_EOL; | ||
| stable_marriage($men, $women); | ||
|
|
||
| $married = stable_marriage($men, $women); |
There was a problem hiding this comment.
I don't understand why you changed stable_marriage to return. You could just use foreach ($women as $woman) in the output loop. In fact, $women and $married have the exact same contents. That's actually how every other implementation does it.
There was a problem hiding this comment.
yes, you are right. I forgot that those are objects, which goes by reference and treated it like arrays which are copies.
return fix
Worked as intended on provided sample 💃