Refactor BaseMetricService (#17) - #37
Conversation
…ass with static methods to functions. Updated the related import statements and function invokation lines in backend/domain/services files, including tags_service.py, difficulty_distribution.py, and abandoned_problem_service.py.
…ssions.py files to correctly call the functions in backend/domain/services/base.py. Change due to shift from BaseMetricService class with static methods to standalone functions.
deyna256
left a comment
There was a problem hiding this comment.
You did a great job! But pulling private functions out of the services package seems very strange to me. I have an idea about refactoring the private functions - let's make an abstraction which will encapsulate a bunch of Problem objects. It can have both of our private methods to do some filtering with the Problem objects it has. What do you think?
|
Hello @deyna256 - thank you for the feedback. I can adjust, but would like to confirm my understanding of your proposed solution before I make further changes: Current
Future
Please let me know if I've missed anything or misunderstood. |
|
@sonnenco exactly! let's do it! |
…es + test files to accommodate.
|
|
||
| @staticmethod | ||
| def _deduplicate_problems(submissions: List[Submission]) -> List[Submission]: | ||
| def _deduplicate_problems(self) -> List[Submission]: |
There was a problem hiding this comment.
In Python, prefixing a method name with an underscore (_) is a convention used to signal that the method is intended to be private (i.e., not part of the public API). But in our case methods: _deduplicate_problems and _filter_successful_submissions are public API of SubmissionProcessor class, rename them
|
|
||
| from backend.domain.models.codeforces import Submission | ||
|
|
||
| class SubmissionProcessor: |
There was a problem hiding this comment.
Why do we name the class like this? A developer cannot understand its meaning if they don't read its methods, so we should rename it.
|
@sonnenco also you need to fix the checks: |
…m private to public functions. Retain original submissions and perform operations on working copy.
…me in ARCHITECTURE.md. Removed class reference in __init__.py as SubmissionCollection is a helper.
|
backend testing
frontend testing
Docker container builds fine locally ( Awaiting validation from workflows to double check results. |
|
Thank you for your patience @deyna256. I've addressed the feedback as best I can. Re-requesting review. |
…dification within class methods. Updated method calls in associated backend services and testing files.
Issue
#17
Changes
Refactored /backend/domain/services/base.py from BaseMetricService class with static methods to standalone functions.
Removed import and use of BaseMetricService class from backend/domain/services/abandoned_problems_service.py due to lack of invocation of _deduplicate_problems or _filter_successful_submissions.
Updated the import statements and use of BaseMetricService in several files to point to the standalone functions, including:
Testing
The following tests come out identical to the main branch:
Please let me know if there are any questions or concerns.