Refactor filterAndSortUids to reduce complexity in src/user/search.js #177
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RogerJinIS
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
P1B: Starter Task: Refactoring PR
1. Issue
Link to the associated GitHub issue:
#119
Full path to the refactored file:
src/user/search.js
What do you think this file does?
This file handles functionality related to user search on the backend of NodeBB, including finding users, applying filters like group or banned status, sorting results, and returning them to the UI. Overall, this implements all the logic related to searching for specific users.
What is the scope of your refactoring within that file?
I refactored the original filterAndSortUids function by breaking it into smaller helper functions without changing its original functionality, inputs, and outputs, so the caller functions do not need to change their logic.
Which Qlty‑reported issue did you address?
Qlty reported a high complexity issue in filterAndSortUids with a complexity count of 11 before refactoring.
2. Refactoring
How did the specific issue you chose impact the codebase’s maintainability?
The original function was doing too many things at once (took on too many responsibilities), making it harder to read, reason, and modify. It would make it harder for future developers to understand and expand on the codebase.
What changes did you make to resolve the issue?
I decomposed filterAndSortUids into multiple small, well-named helper functions, corresponding to each responsibility to a single functionality, making the factored version now contain clear modularity rather than deeply nested logic.
How do your changes improve maintainability? Did you consider alternatives?
The refactoring has improved readability and allows for the potential reuse of the smaller helper functions, so the same functionality would not be written redundantly if needed again in the future. As alternatives, I considered different ways to break down the original function into smaller helper functions.
3. Validation
How did you trigger the refactored code path from the UI?
Attach a screenshot of the logs and UI demonstrating the trigger.
(If you refactored a public/src/ file (front-end related file), watch logging via DevTools (Ctrl+Shift+I to open and then navigate to the 'Console' tab). If you refactored a src/ file, watch logging via ./nodebb log. Include the relevant UI view. Temporary logs should be removed before final commit.)
Attach a screenshot of
qlty smells --no-snippets <full/path/to/file.js>showing fewer reported issues after the changes.Before:
After:
