Rust icon indicating copy to clipboard operation
Rust copied to clipboard

feat: Added Stable Matching Algorithm

Open BKarthik7 opened this issue 1 year ago • 3 comments

feat: Added Stable Matching Algorithm

Description

This commit introduces the Stable Matching Algorithm, commonly known as the Gale-Shapley Algorithm, to the codebase. It solves the problem of matching two sets (e.g., men and women) based on preference lists, ensuring that the final matching is stable—meaning no two individuals would rather be paired with each other than with their current partners.

Type of change

  • [x] New feature (non-breaking change which adds functionality)

Checklist:

  • [x] I ran bellow commands using the latest version of rust nightly.
  • [x] I ran cargo clippy --all -- -D warnings just before my last commit and fixed any issue that was found.
  • [x] I ran cargo fmt just before my last commit.
  • [x] I ran cargo test just before my last commit and all tests passed.
  • [x] I added my algorithm to the corresponding mod.rs file within its own folder, and in any parent folder(s).
  • [x] I added my algorithm to DIRECTORY.md with the correct link.
  • [x] I checked COUNTRIBUTING.md and my code follows its guidelines.

BKarthik7 avatar Oct 05 '24 15:10 BKarthik7

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.36%. Comparing base (209c1b4) to head (d9b140c). Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #806      +/-   ##
==========================================
+ Coverage   95.31%   95.36%   +0.05%     
==========================================
  Files         310      311       +1     
  Lines       22430    22673     +243     
==========================================
+ Hits        21380    21623     +243     
  Misses       1050     1050              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Oct 05 '24 15:10 codecov-commenter

Please add the missing tests. We are aiming to 100% coverage.

done it reached 100% convergence

BKarthik7 avatar Oct 05 '24 17:10 BKarthik7

@vil02 @imp2002 ready for review

BKarthik7 avatar Oct 05 '24 19:10 BKarthik7

@sozelfist could you take a look? Do you have some reasonable idea how to refactor stable_matching into smaller pieces?

vil02 avatar Oct 06 '24 18:10 vil02

@sozelfist could you take a look? Do you have some reasonable idea of how to refactor stable_matching into smaller pieces?

Sure, I will propose decomposing the stable_matching function into helper functions that handle the separated logic and make the main function more streamlined and focused.

sozelfist avatar Oct 07 '24 07:10 sozelfist

Btw, why don't we write tests using macros? It makes the test more compact and we can suggest adding some edge tests.

sozelfist avatar Oct 07 '24 08:10 sozelfist

What do you think about the refactor, @vil02 and @BKarthik7?

sozelfist avatar Oct 07 '24 08:10 sozelfist

What do you think about the refactor, @vil02 and @BKarthik7?

I understood the refactor and is nice but if a newbie wants to understand the code i thought the original version was more readable it's just my opinion I am open to hear your point of view too. I implemented the changes and pushed it.

BKarthik7 avatar Oct 07 '24 08:10 BKarthik7

The original version might be easier for beginners due to its linear structure, but the refactored version is better in terms of maintainability and best practices.

sozelfist avatar Oct 07 '24 08:10 sozelfist

@sozelfist done is there any thing else

BKarthik7 avatar Oct 07 '24 08:10 BKarthik7

I just proposed the refactored version of your code. Maybe @vil02 would have something to propose.

sozelfist avatar Oct 07 '24 08:10 sozelfist

Btw, why don't we write tests using macros? It makes the test more compact and we can suggest adding some edge tests.

You can consider this one for tests

sozelfist avatar Oct 07 '24 08:10 sozelfist