Repository navigation
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ca067f9 to
abe319f
Compare
ed55e0a to
744b77b
Compare
| * The industry a member is in or wants to get into, from the sign-up dropdown stored in {@code Member.industryPref}. | ||
| * Comment - The current match preference dropdown includes description, the mock data does not | ||
| */ | ||
| public enum Industry { |
There was a problem hiding this comment.
also for the future - but i feel like it might be a good idea to eventually have a negative industry match - like someone wanting to talk to someone out of their industry too. we'd have to adjust the signup form as well, but just a thought.
| return NO_PREFERENCE; | ||
| } | ||
| String trimmed = industryPref.trim(); | ||
| for (Industry industry : values()) { |
There was a problem hiding this comment.
you can just do Industry.valueOf(industryPref) here
744b77b to
cad8a11
Compare
abe319f to
a4241b3
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
|
| return isCompatible(prefA, prefB) ? COMPATIBLE_BONUS : CONFLICT_PENALTY; | ||
| } | ||
|
|
||
| private static boolean isCompatible(MatchPreference prefA, MatchPreference prefB) { | ||
| return (prefA == MatchPreference.MENTOR && prefB == MatchPreference.MENTEE) | ||
| || (prefA == MatchPreference.MENTEE && prefB == MatchPreference.MENTOR) | ||
| || (prefA == MatchPreference.PEER && prefB == MatchPreference.PEER) | ||
| || (prefA == MatchPreference.NO_PREFERENCE || prefB == MatchPreference.NO_PREFERENCE); |
There was a problem hiding this comment.
The Javadoc says a member with no preference is neutral, but isCompatible returns true whenever either side is NO_PREFERENCE, so those pairs get COMPATIBLE_BONUS (+20) instead of 0. Missing or unrecognized matchPref values parse to NO_PREFERENCE, so they also get +20.
The greedy pairer then ranks "no preference" pairs level with true Mentor/Mentee and Peer/Peer pairs, and above pairs that merely lack a conflict. The CLI's "no preference" count is also always 0, because such pairs are counted as compatible.
Fix: return 0 when either side is NO_PREFERENCE, and keep +20 or -20 only for specific preferences.
if (prefA == NO_PREFERENCE || prefB == NO_PREFERENCE) return 0;
return isCompatible(prefA, prefB) ? COMPATIBLE_BONUS : CONFLICT_PENALTY;| return isCompatible(prefA, prefB) ? COMPATIBLE_BONUS : CONFLICT_PENALTY; | |
| } | |
| private static boolean isCompatible(MatchPreference prefA, MatchPreference prefB) { | |
| return (prefA == MatchPreference.MENTOR && prefB == MatchPreference.MENTEE) | |
| || (prefA == MatchPreference.MENTEE && prefB == MatchPreference.MENTOR) | |
| || (prefA == MatchPreference.PEER && prefB == MatchPreference.PEER) | |
| || (prefA == MatchPreference.NO_PREFERENCE || prefB == MatchPreference.NO_PREFERENCE); | |
| if (prefA == MatchPreference.NO_PREFERENCE || prefB == MatchPreference.NO_PREFERENCE) { | |
| return 0; | |
| } | |
| return isCompatible(prefA, prefB) ? COMPATIBLE_BONUS : CONFLICT_PENALTY; | |
| } | |
| private static boolean isCompatible(MatchPreference prefA, MatchPreference prefB) { | |
| return (prefA == MatchPreference.MENTOR && prefB == MatchPreference.MENTEE) | |
| || (prefA == MatchPreference.MENTEE && prefB == MatchPreference.MENTOR) | |
| || (prefA == MatchPreference.PEER && prefB == MatchPreference.PEER); | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
@Allimonae this is a valid comment - maybe we should assign 0 points to the score if its no preference instead?
| } | ||
| String trimmed = matchPref.trim(); | ||
| for (MatchPreference preference : values()) { | ||
| if (trimmed.regionMatches(true, 0, preference.label, 0, preference.label.length())) { |
There was a problem hiding this comment.
same as above for valueOf
| return isCompatible(prefA, prefB) ? COMPATIBLE_BONUS : CONFLICT_PENALTY; | ||
| } | ||
|
|
||
| private static boolean isCompatible(MatchPreference prefA, MatchPreference prefB) { | ||
| return (prefA == MatchPreference.MENTOR && prefB == MatchPreference.MENTEE) | ||
| || (prefA == MatchPreference.MENTEE && prefB == MatchPreference.MENTOR) | ||
| || (prefA == MatchPreference.PEER && prefB == MatchPreference.PEER) | ||
| || (prefA == MatchPreference.NO_PREFERENCE || prefB == MatchPreference.NO_PREFERENCE); |
There was a problem hiding this comment.
@Allimonae this is a valid comment - maybe we should assign 0 points to the score if its no preference instead?
| public int score(Member memberA, Member memberB) { | ||
| Industry industryA = Industry.fromIndustryPref(memberA.getIndustryPref()); | ||
| Industry industryB = Industry.fromIndustryPref(memberB.getIndustryPref()); | ||
| return industryA.isSpecific() && industryA == industryB ? SAME_INDUSTRY_BONUS : 0; |
There was a problem hiding this comment.
if were being biased towards same industry for those who want it, shouldnt we apply a small negative penalty if both industries are specific and different?




Overview
This adds the scoring-based matching algorithm, which replaces the purely random pairing used by the matching CLI.
Greedy pairing
GreedyPairerscores every possible pair of members, then takes the highest-scoring pairs first and skips any pair that includes someone already taken.Randombefore scoring, so ties break randomly but reproducibly. With no scorers, the result is identical toRandomPairerfor the same seed.Scorers, each added as a layer on top of the previous ones:
HistoryScorerapplies a-100penalty to any pair matched in a previous cycle, regardless of order. Repeats only happen when there is no alternative.MatchPrefScorergives+20for compatible preferences (Mentor with Mentee, Peer with Peer, or either member having no preference) and-20for conflicting ones. The history penalty outweighs the best preference bonus, so preferences never cause a repeat.IndustryScorergives+10when both members share a specific industry. "Other", "No Preference" and differing industries are neutral, so industry only breaks ties between pairs the earlier layers score equally.Preference parsing
IndustryparsesMember.industryPref, ignoring case and whitespace. Missing or unrecognized values count as "No Preference".MatchPreferenceparsesMember.matchPref. The stored value is the full dropdown text (e.g. "Mentor - I am looking for guidance ..."), so it is recognized by its leading label. Missing or unrecognized values count as "No Preference".Matching CLI
MatchingClinow builds the scorer list for the selected layer and every layer before it, and runsGreedyPairerinstead ofRandomPairer.