Skip to content

Sheffield | 26-ITP-Jan | Mahmoud Shaabo | Sprint 1 | Module-Data-Groups#1014

Closed
mahmoudshaabo1984 wants to merge 2 commits intoCodeYourFuture:mainfrom
mahmoudshaabo1984:sprint-1-exercises
Closed

Sheffield | 26-ITP-Jan | Mahmoud Shaabo | Sprint 1 | Module-Data-Groups#1014
mahmoudshaabo1984 wants to merge 2 commits intoCodeYourFuture:mainfrom
mahmoudshaabo1984:sprint-1-exercises

Conversation

@mahmoudshaabo1984
Copy link
Copy Markdown

Completed all mandatory exercises for Sprint 1 in the following directories: fix, implement, and refactor.

Progress Checklist:

  • fix/median.js: Fixed median calculation, added guard clause for non-array inputs, and handled NaN edge cases after reviewing JavaScript documentation.
  • implement/max.js: Implemented logic using Math.max and spread syntax with NaN filtering to ensure robust numerical operations.
  • implement/sum.js: Built the summation logic using .reduce() while ignoring non-numeric values and ensuring NaN safety.
  • implement/dedupe.js: Cleaned up duplicates using the modern Set object.
  • refactor/includes.js: Refactored traditional for-loops into modern for...of syntax for better readability.
  • Testing: Verified that all 24 tests passed successfully using npm test.

Personal Note for CJ:

Hi CJ,
I have ensured that all checkboxes follow the [x] syntax. During this Sprint, I spent some time researching JavaScript documentation regarding numerical types, which led me to implement additional checks for NaN values to make my functions more robust. Navigating the test results with NVDA helped me identify a few syntax errors early on. Everything is now passing and aligned with the required best practices. Looking forward to your feedback!

@mahmoudshaabo1984 mahmoudshaabo1984 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 15, 2026
@mahmoudshaabo1984
Copy link
Copy Markdown
Author

Hi CJ,

Thank you for the guidance in the previous sprints. For this Sprint 1 submission, I have double-checked all logic and ensured that edge cases, particularly NaN values in numerical arrays, are handled correctly by implementing stricter filtering. I arrived at these improvements after researching JavaScript's handling of non-numeric types and the behavior of Math.max and .reduce() on MDN.

I’ve also ensured that my PR title and description follow the required formatting and that all task checkboxes are marked. Ready for your review and feedback!

Best regards,
Mahmoud Shaabo

Copy link
Copy Markdown
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. I only have a few suggestions.

Comment thread Sprint-1/implement/dedupe.test.js
Comment thread Sprint-1/implement/dedupe.test.js Outdated
Comment thread Sprint-1/implement/max.test.js
Comment thread Sprint-1/implement/sum.test.js
@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 18, 2026
@mahmoudshaabo1984
Copy link
Copy Markdown
Author

Hi CJ,
I have updated the test files in the sprint-1/implement folder based on your feedback. Specifically, I have made the following changes:
. 1
sum.test.js: Switched to toBeCloseTo() to handle floating-point precision correctly.
. 2
dedupe.test.js: Added a test case for mixed-type arrays and included an immutability check to ensure the function returns a new array reference.
. 3
max.test.js: Added a test case to ensure numeric strings (like "300") are ignored and not coerced into numbers.
I also fixed the Markdown syntax for the checkboxes in the PR description as requested. All 47 tests are now passing. Ready for your review!
Best regards,
Mahmoud

@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Mar 28, 2026

Changes look good.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Mar 28, 2026
@illicitonion
Copy link
Copy Markdown
Member

Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants