Skip to content

Conversation

HoussamLh
Copy link

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

1. Sum of Numbers (sum.js)

  • Implements a function to sum numerical elements of an array.
  • Ignores non-numerical values.
  • Handles empty arrays, negative numbers, decimals.
  • Tests included in sum.test.js.

2. Maximum Number (max.js)

  • Finds the largest numerical element in an array.
  • Ignores non-numeric values.
  • Handles empty arrays, negative numbers, decimals.
  • Tests included in max.test.js.

3. Deduplicate Array (dedupe.js)

  • Removes duplicates from arrays, preserving the first occurrence.
  • Works for arrays with strings or numbers.
  • Handles empty arrays.
  • Tests included in dedupe.test.js.

4. Median Calculation (calculateMedian.js)

  • Returns the median of numeric elements.
  • Filters out non-numeric values.
  • Handles sorted, unsorted, odd/even length arrays.
  • Tests included in median.test.js.

5. Includes (includes.js)

  • Checks if an array includes a target value.
  • Refactored to use for...of loop.
  • Handles empty arrays, multiple occurrences, null values.
  • Tests included in includes.test.js.

6. Advent of Code 2018 Day 1 (Chronal Calibration)

  • Calculates the resulting frequency after applying all frequency changes.
  • Finds the first frequency that is reached twice.
  • Input file included (day1-input.txt).
  • Solution implemented in day1.js (or relevant file).
  • Tests/console output included for verification.

Questions

  1. Are the test cases covering all meaningful edge cases for sum, max, dedupe, includes, and calculateMedian?
  2. For includes, is the for...of loop implementation preferred, or should we use higher-order array methods?

- Properly calculates median for odd and even-length arrays
- Handles unsorted arrays
- Filters out non-numeric values
- Returns null for invalid or empty inputs
- Ensures the input array is not modified
- Added dedupe.js to remove duplicate elements from arrays while preserving first occurrence
- Handles empty arrays, arrays with numbers, strings, or mixed values
- Added dedupe.test.js covering:
  • empty array
  • arrays with no duplicates
  • arrays with duplicate strings, numbers, and mixed values
- Ensures original array is not modified
- Added max.js to find the largest numerical element in an array
- Ignores non-numeric values
- Handles empty arrays, negative numbers, decimal numbers, and mixed-type arrays
- Added max.test.js covering:
  • empty array
  • single-element array
  • positive and negative numbers
  • decimal numbers
  • arrays with non-number values
  • arrays with only non-number values
- Added sum.js to calculate the total sum of numerical elements in an array
- Ignores non-numeric values
- Handles empty arrays, negative numbers, decimal numbers, and mixed-type arrays
- Added sum.test.js covering:
  • empty array
  • single-element array
  • positive and negative numbers
  • decimal numbers
  • arrays with non-number values
  • arrays with only non-number values
- Refactored includes.js to iterate over array elements using a for...of loop
- Preserves original behavior:
  • returns true if target is found
  • returns false if target is not found
- All existing tests in includes.test.js remain passing
- Implemented calculation of final frequency.
- Implemented detection of first repeated frequency.
- Reads input from input.txt and logs results to console.
@HoussamLh HoussamLh added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 9, 2025
@LonMcGregor LonMcGregor added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Sep 10, 2025
Copy link

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

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

Good start on this sprint's tasks, I have spotted a few areas where you could improve code further

Did you mean to make changes to the package.json files? We try to keep pull requests specific to just the files in the feature we are changing.

q1: For edge cases, the idea is that you've covered what is correct and most of the common "wrong" values or types that might be given. For most ttasks would be difficult to find every possible edge case to test. It looks to me like you have done a good job of finding possible test cases in these tasks.

q2: The for..of you have used is correct.

const median = list.splice(middleIndex, 1)[0];
return median;
// Sort numbers in ascending order
numbers.sort((a, b) => a - b);

Choose a reason for hiding this comment

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

Do you need to include this anonymous sorting function as an argument to sort?

Copy link
Author

Choose a reason for hiding this comment

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

Hello LonMcGregor,

Thank you for the feedback!

I double-checked the sort function—using (a, b) => a - b is necessary to sort numbers numerically; otherwise, JavaScript sorts them as strings, which would break the median calculation.
I also removed the splice approach from my earlier attempt to avoid mutating the array—returning numbers**[middleIndex]** directly works cleanly for odd-length arrays.

@LonMcGregor LonMcGregor 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. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Sep 10, 2025
@HoussamLh HoussamLh added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants