Refactor (src/groups/membership.js): reduce complexity membership exports

[rebeccals05]

P1B: Starter Task: Refactoring PR

You are not permitted to use generative AI services (e.g., ChatGPT) to compose the answers.
Any such use will be treated as a violation of academic integrity.

1. Issue

Link to the associated GitHub issue:
#202

Full path to the refactored file:
src/groups/membership.js

What do you think this file does?
I believe this file handles users' memberships to groups within NodeBB. It checks whether or not a user belongs to a particular group before allowing them access to the group.

What is the scope of your refactoring within that file?
I touched many functions in membership.js. The ones I edited to reduce return statements for one of the smells were Groups.getMembers, Groups.getMembersOfGroups, and Groups.getMemberCount. I refactored Groups.isMember, Groups.isMembers, isMemberOfGroups, Groups.isMembersOfGroupList, and getGroupNames.

Which Qlty‑reported issue did you address?
Function with high complexity (count = 24): exports

2. Refactoring

How did the specific issue you chose impact the codebase’s adaptability?
The issue I chose was high complexity, which makes the code harder to understand, and thus, harder and riskier to change without creating bugs. Highly complex code is also harder for new software developers to maintain because they must first spend time understanding it.

What changes did you make to resolve the issue?
I removed return statements when I could to reduce the "Function with many returns" smell as well. I also replaced a nested loop with clearer functions and broke down nested logic repeated across different functions into helper functions at the beginning of the file.

How do your changes improve adaptability? Did you consider alternatives?
I think the code is a little clearer to read for the first time and understand than it was before, due to the inclusion of helper functions, which allow the code to be read similarly to English. At first, I was just going to address the "many returns" smell, but as I was removing the multiple returns within the if statements and returning at the end of the code, my complexity was increasing because I was adding more and more conditionals. Instead, I decided it was probably in the best interest of the code's adaptability to focus on the complexity, rather than the returns.

3. Validation

How did you trigger the refactored code path from the UI?
I triggered the code path in the NodeBB UI by clicking into the "Groups" tab along the top of the screen. This action triggered the first set of console.log messages. Then, I clicked into the "Administrators Group," which triggered the second set of messges in the log.

[coveralls]

Pull Request Test Coverage Report for Build 17482336362

Details

• 57 of 57 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 78.542%

---

Totals
Change from base Build 17435863312:	0.01%
Covered Lines:	24701
Relevant Lines:	29611

---

💛 - Coveralls