Refactor (src/user/picture.js): reduce complexity and improve export … #162
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.
…structure
P1B: Starter Task: Refactoring PR
Feel free to delete this text at the top after filling out the template.
1. Issue
(#152)
(https://github.com/sliang-code/NodeBB/tree/refactor-picture-exports-complexity)
This file is designed to handle the profile picture uploads and related operations (like chopping and resizing)
I changed the structure of this file without modifying the content of any detailed function. I created the functions initCoverMethods, initProfileImageMethods, initGetters and moved the User.updateCoverPosition, User.updateCoverPicture, User.removeCoverPicture, User.uploadCroppedPictureFile, User.uploadCroppedPicture, User.removeProfileImage, User.getAllowedProfileImageExtensions, User.getAllowedImageTypes, User.getLocalCoverPath, User.getLocalAvatarPath inside and moved the function declarations validateUpload, generateProfileImageFilename, getPicturePath, deleteCurrentPicture, deletePicture, convertToPNG to reduce the number of returns in the module.exports and lower the complexity.
(Name specific functions/blocks/regions touched.)
13 Function with many returns (count = 15): exports -- main issue to refactor
(13 Function with high complexity (count = 21): exports) -- (did not intend to refactor at first but also fixed at the same time as the first is refactored)
2. Refactoring
The execution flow is complex and hard to track when the code has 15 return points. This makes the program hard to predict. Refactoring this issue can make the flow more clear and easy to understand.
I changed the structure of this file by removing the function declarations out of module.exports and packed the User."Name" functions into several large function declarations to make it into independent modules. Also I changed some function inputs to solve the adaptability (User).
Restructuring the function declarations makes the code more readable and also easier to understand the return flow. Packing the User."Name" functions into larger functions based on their functionality also make the code more readable.
3. Validation
I added the code "Console_log('Sihan Liang')" at the beginning of several functions (Picture input functions). I tried to trigger the function by signing an account and upload or change a picture of the user. However I failed as the upload/change button does not respond to me. I guess that it might be some front-end failures as I was also not able to upload any pictures in "making posts" module. Therefore, I add the "Console_log('Sihan Liang')" right into

module.exports = function (User) {
initGetters(User);
initCoverMethods(User);
initProfileImageMethods(User);
};
and the following screenshot shows that the program indeed reaches such line when executing
and the following is the console_log() screenshots



This is the qlty after refactoring
