Decreasing high complexity in src/socket.io/admin/rooms.js:90 function SocketRooms.getLocalStats #159
+50
−38
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
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
#123 (comment)
Full path to the refactored file:
src/socket.io/admin/rooms.js
What do you think this file does?
I think what this file does is that it fetches the amount of online users and other metrics for the admin dashboard to then fetch and represent in graphs and numbers.
What is the scope of your refactoring within that file?
I refactored the "SocketRooms.getLocalStats" functions where I took some high complextiry parts of the code and changed into some helper functions which make it easier to read. I created two new fuhelper functions (processRoomData and createEmptyStatsObject) in order to do so. They span from line 90 to line 147
Which Qlty‑reported issue did you address?
Function with high complexity (Count = 10)
2. Refactoring
How did the specific issue you chose impact the codebase’s maintainability?
The function I chose was cluttery and had a lot of nester ifs and for loops which don't make sense to someone who looks at it randomly.
What changes did you make to resolve the issue?
When someone looks at the code, it looks much less cluttery with the new helper functions. I also added some comments to make it more readable and maintainable.
How do your changes improve maintainability? Did you consider alternatives?
They clearly break down the functionality of the function into more clear helper functions, all of which have good names and easily understandable code. The comments also increase readability.
3. Validation
How did you trigger the refactored code path from the UI?
I wasn't able to trigger the function directly from the UI or through doing actions. I tinkered around the website through multiple instances of NodeBB at the same time (since it is a admin feature that has analytics). However none of them triggered anything. I tried to ask an LLM where this code could possibly be triggered and its ideas were also not working. So for now I am showing when it gets triggered in the npm run test (which means its running my code).
Attach a screenshot of the logs and UI demonstrating the trigger.
Attach a screenshot of
qlty smells --no-snippets src/socket.io/admin/rooms.jsshowing fewer reported issues after the changes.Before
After