Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract Mutation Strategy from RankInvariantChecker and Update Tests #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BashithaShamila
Copy link

In this pull request, I have refactored the RankInvariantChecker class by extracting the mutation strategy into a separate MutationStrategy interface. This enhancement promotes greater modularity and flexibility, allowing developers to easily implement and integrate custom mutation strategies without modifying the core RankInvariantChecker logic. Additionally, I introduced a default mutation strategy implementation and updated the relevant tests to ensure seamless functionality with the new structure. These changes improve the maintainability and extensibility of the codebase, facilitating future enhancements and adaptations.

@leliel12
Copy link
Collaborator

leliel12 commented Sep 20, 2024

  • The tests and the API documentation of the new strategies are needed
  • In other hand maybe the strategy can be implemented as a function (like all the strategy patterns in python). a New class is unnecessary

I imagine something like:

def my_mutation(**kwargs):  # we must decide with "kwargs" are necesary
   ....
   
rrt1 = RankInvariantChecker(mutation_strategy=my_mutation)

My suggestions are to reduce the number of new modules, reduce the number of new documentation and obviously reduce the number of testing. Making this code more maintainable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants