Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored code in src/topics/posts.js #56

Open
wants to merge 7 commits into
base: f24
Choose a base branch
from
Open

Conversation

njouud
Copy link

@njouud njouud commented Sep 2, 2024

Added two helper functions for addEventStartEnd (line 78) to reduce cognitive complexity from 23 to the 15 allowed.

Helper 1: setEventTimes, to be called in the loop for each post object in the postData array.
Helper 2: handleLastPost, to be called at the end of the main function. All the nested if statements for dealing with the last post in a topic are extracted from each other.

Testing and justification for missing tests: the function already has a high percentage of coverage, with only a few already existing lines that are missing tests. I have not added more tests as the function is being tested implicitly, the actual function is not being called in the test files even though it has a high percentage of coverage. Because of this I was not sure how to add more tests, I tried to add tests but I kept getting errors.

@coveralls
Copy link

coveralls commented Sep 2, 2024

Pull Request Test Coverage Report for Build 10873863039

Details

  • 15 of 22 (68.18%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 82.624%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/topics/posts.js 15 22 68.18%
Files with Coverage Reduction New Missed Lines %
src/meta/errors.js 1 76.74%
Totals Coverage Status
Change from base Build 10550029021: -0.05%
Covered Lines: 22324
Relevant Lines: 25591

💛 - Coveralls

Copy link

sonarcloud bot commented Sep 15, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants