Refactor(src/user/profile.js): reduce complexity in changePassword#186
Open
connorcarpenter15 wants to merge 2 commits intoCMU-313:mainfrom
Open
Refactor(src/user/profile.js): reduce complexity in changePassword#186connorcarpenter15 wants to merge 2 commits intoCMU-313:mainfrom
connorcarpenter15 wants to merge 2 commits intoCMU-313:mainfrom
Conversation
Reduce the complexity of the `User.changePassword` function by consolidating validation logic in the helper function `validatePasswordRequest`.
Auto-formatter changed parens and spacing. Revert back to previous formatting for linter.
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
Link to the associated GitHub issue: #185
Full path to the refactored file:
src/user/profile.jsWhat do you think this file does?
(Your answer does not have to be 100% correct; give a reasonable, evidence‑based guess.)
This file implements functionality for interacting with user objects, such as validating attributes (i.e. birthday, email), updating profile information (i.e. username, email), and changing passwords.
What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)
The scope of this refactoring involved the
User.changePasswordfunction, specifically the validation logic at the start of the function.Which Qlty‑reported issue did you address?
(Name the rule/metric and include the BEFORE value; e.g., “Cognitive Complexity 18 in render()”.)
Function with high complexity (count = 10)
2. Refactoring
How did the specific issue you chose impact the codebase’s maintainability?
The incorporation of extensive validation logic within the
User.changePasswordfunction itself decreases readability and understanding of the main purpose of the function, making it harder to debug and modify.What changes did you make to resolve the issue?
I separated the validation logic into a helper function.
How do your changes improve maintainability? Did you consider alternatives?
Separating this logic into a helper function increases modularity and makes it easier to debug and modify this function in the future. This is the most straightforward and widely-used method for this use case, so no alternatives were considered.
3. Validation
How did you trigger the refactored code path from the UI?
I navigated to the user profile settings, selected to change password, and inputted a new password in the given fields.
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.