Solution

[Marcin123456t]

No description provided.

[mateacademy-ai-mentor]

Review Summary

Great work on this implementation of reduce! The code is clean, and your tests cover the most common use cases effectively. You've correctly handled the logic for using and omitting an initialValue.

However, there are two critical ways the function deviates from the standard Array.prototype.reduce behavior that need to be fixed before this can be approved. These are important for creating a true polyfill that developers can rely on.

Here are the required changes:

1. The callback argument is not validated.

The native reduce method throws a TypeError if the provided callback is not a function. Your implementation will proceed and cause a less specific error later. Please add a check at the beginning of your function.

• File: src/reduce.js
• Suggestion: You can add a check like this at the top of your function:

  if (typeof callback !== 'function') {
    throw new TypeError(callback + ' is not a function');
  }

2. Sparse arrays are not handled correctly.

Your for loop iterates over all indices, including empty slots in sparse arrays (e.g., [1, , 3]). The native reduce method skips these empty slots entirely. This causes your function to produce incorrect results for these arrays.

• File: src/reduce.js (inside your loop)
• Suggestion: Before executing the callback, you should check if the index actually exists on the array. The in operator is perfect for this:

  // Inside your for loop
  if (i in this) {
    // ... your existing logic to call the callback ...
  }

Once you've addressed these two issues, your implementation will be much more robust and aligned with the official specification. You're very close—keep up the great work!

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

The provided callback should be validated to ensure it's a function. The native Array.prototype.reduce throws a TypeError if the callback argument is not a function. Please add this check at the beginning of your function.

[mateacademy-ai-mentor]

Review Summary

Great job on building a custom reduce implementation and writing a solid set of tests for it! You've correctly handled many of the core use cases and edge scenarios.

However, there are a couple of critical functional differences between your implementation and the standard Array.prototype.reduce that need to be addressed before this can be approved.

Here are the required changes:

1. Incorrect Handling of Sparse Arrays
   • Issue: Your reduce function doesn't correctly handle sparse arrays (arrays with empty slots, like [1, , 3]). The native method skips these empty slots, but your implementation processes them as undefined, which can lead to incorrect results (e.g., NaN when summing numbers).
   • To Fix: You'll need to modify your loop to check if an element actually exists at a given index before processing it. A common way to do this is with Object.prototype.hasOwnProperty.call(this, i).
   • Related Test: It's also a good idea to add the test case for this scenario, as suggested in the review comments:

it('should skip empty slots in sparse arrays', () => {
const sparseArr = [1, , 3]; // eslint-disable-line no-sparse-arrays
const result = sparseArr.reduce2((acc, curr) => acc + curr, 0);
expect(result).toBe(4);
});

2.  **Missing Callback Type Validation**
    *   **Issue**: The native `reduce` throws a `TypeError` if the provided callback is not a function. Your implementation is missing this validation. For example, `[].reduce('not a function', 0)` should throw an error but currently returns the initial value `0`.
    *   **To Fix**: Add a check at the very beginning of your function to ensure the `callback` parameter is a function.
        ```javascript
if (typeof callback !== 'function') {
  throw new TypeError(`${callback} is not a function`);
}

Once you've fixed these two issues, your implementation will be much more robust and compliant with the JavaScript specification. You're very close! Keep up the great work.

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

It's a good practice to test how the function handles sparse arrays (arrays with empty slots). The native Array.prototype.reduce skips these empty slots, but the current implementation does not, which can lead to unexpected behavior. Adding a test for this case would highlight this discrepancy.

it('should skip empty slots in sparse arrays', () => {
  const sparseArr = [1, , 3]; // eslint-disable-line no-sparse-arrays
  const result = sparseArr.reduce2((acc, curr) => acc + curr, 0);
  expect(result).toBe(4);
});

[mateacademy-ai-mentor]

The test for callback arguments only covers the scenario where an initial value is provided. It would be beneficial to add another test case to verify that the correct arguments (accumulator, currentValue, currentIndex, array) are passed to the callback when no initial value is given.

it('should pass correct arguments to callback without initial value', () => {
  const arr = [10, 20, 30];
  const mockCallback = jest.fn((acc, curr) => acc + curr);

  arr.reduce2(mockCallback);

  expect(mockCallback.mock.calls.length).toBe(2);
  expect(mockCallback).toHaveBeenCalledWith(10, 20, 1, arr);
  expect(mockCallback).toHaveBeenCalledWith(30, 30, 2, arr);
});

[mateacademy-ai-mentor]

Review Summary

Great work on this! I'm approving this submission because you've built a solid implementation of reduce that handles all the core functionality correctly, and your test suite is very well-structured. This is an excellent foundation to build upon.

I want to highlight a couple of key areas for improvement that focus on meeting the official JavaScript specification. Addressing these will make your implementation even more robust.

Implementation Feedback (src/reduce.js)

1. Handling Sparse Arrays

Your implementation currently treats empty slots in sparse arrays as undefined, which is a common mistake. The official Array.prototype.reduce specification requires that these empty slots are skipped entirely.

• The Issue: In src/reduce.js, your for loop iterates over every index. For an array like [1, , 3], your code calls the callback for the empty slot at index 1, passing undefined as the value.
• Why it Matters: The native reduce method would not invoke the callback for index 1 at all. It only runs for indices that have actually been assigned a value.
• A Hint for Fixing: You can check if an index truly exists as a property on the array before executing the callback. The in operator is perfect for this. Consider wrapping your callback execution in a condition like this:

  // Inside your for loop
  if (i in this) {
    // ... call the callback here
  }

2. Handling a Sparse Array Without an Initial Value

There's a subtle bug when no initialValue is provided for a sparse array that starts with an empty slot (e.g., [, 1, 2]).

• The Issue: Your code uses this[0] as the starting value. For [, 1, 2], this[0] is undefined, which becomes the initial accumulator.
• Why it Matters: The native reduce method is smarter; it finds the first existing element in the array to use as the initial value and starts the iteration from the next existing element.
• A Hint for Fixing: When no initialValue is provided, you'll need to loop through the array to find the first index that exists (using the in operator again) to set your initial accumulator and startIndex.

Test Suite Feedback (src/reduce.test.js)

Your tests are thorough, but we can make them even better at catching the kinds of bugs mentioned above.

1. Misleading Test for Sparse Arrays

The test named 'should skip empty slots in sparse arrays' currently passes, but it masks the bug in your implementation.

• The Issue: The test works because the callback you wrote ((acc, curr) => acc + (curr !== undefined && curr !== null ? curr : 0)) is specifically designed to handle the undefined values that your faulty implementation passes. A good test should fail when the implementation is wrong.
• A Hint for Improving: The best way to test this is to verify the behavior, not just the final result. You can use a mock function (jest.fn()) to assert that the callback was called the correct number of times. For [1, , 3], the callback should only be called twice.

  const mockCallback = jest.fn();
  [1, , 3].reduce(mockCallback, 0);
  expect(mockCallback).toHaveBeenCalledTimes(2); // This would fail with your current code

Overall, this is a fantastic effort. You've mastered the core logic, and now you're digging into the important details that define a robust, spec-compliant function. Keep up the great work

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Footnotes

1. Rate AI review example ↩

[mateacademy-ai-mentor]

When no initial value is provided, using this[0] as the starting value is correct for dense arrays. However, for a sparse array that starts with an empty slot (e.g., [, 1, 2]), this[0] would be undefined. The standard reduce finds the first existing element in the array to use as the initial value and starts the iteration from the next element.

[mateacademy-ai-mentor]

This test's name, 'should skip empty slots in sparse arrays', is misleading. The test implementation doesn't actually verify that the callback is skipped for empty slots. It only checks the final computed value, and the callback used ((acc, curr) => acc + (curr !== undefined && curr !== null ? curr : 0)) is specifically designed to handle the undefined values that the current reduce implementation incorrectly passes for empty slots. A more accurate test would use jest.fn() to assert that the callback is not called for the indices of empty slots.