array_method_reduce by robsza1122 · Pull Request #90 · mate-academy/qa_array-method-reduce

robsza1122 · 2025-10-06T15:20:43Z

No description provided.

mateacademy-ai-mentor

Review Summary

I’m requesting changes because core requirements of reduce are not met and tests reinforce incorrect behavior. Good job covering basic reduce cases and keeping the implementation readable.

Critical issues to fix:

src/reduce.js: Lines 13–16 assume this[0] when no initialValue; instead, scan for the first present index (k in this), use it as the initial accumulator, and start at k + 1. If none found, throw a TypeError (native behavior).
src/reduce.js: Line 19 throws a generic Error; replace with new TypeError('Reduce of empty array with no initial value') to match native semantics.
src/reduce.js: Add an explicit typeof callback === 'function' check near the top and throw TypeError if not; cache const len = this.length before iterating; iterate to len and only invoke the callback for indices that exist (if (i in this)). This ensures holes are skipped and length mutations don’t affect visited indices.
src/reduce.test.js: Update the empty-array-without-initialValue expectation to toThrow(TypeError) (not a custom message). Add tests that: (1) verify holes are skipped using a jest.fn() and assert visited indices; (2) confirm empty array WITH initialValue returns that value and callback is not called; (3) cover accumulating into arrays/objects; (4) cover mutation during iteration; (5) optionally verify callback this/receiver behavior is not implicitly bound.

Minor improvement: consider guarding this == null and throwing TypeError to mirror native behavior. With these changes, your implementation and tests will align with native Array.prototype.reduce. Keep up the good work—these adjustments will solidify correctness and spec compliance.

✨ 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.

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.js

    prev = this[0];
  }

+  if (arguments.length < 2 && this.length === 0) {


Behavior when calling reduce without an initial value on an empty or sparse array needs fixing. Right now the code throws a generic Error for empty arrays and assumes the first element is at index 0 when no initialValue is provided (lines 13-16 set prev = this[0] and startIndex = 1). The correct behavior required by the task (and native Array.prototype.reduce) is: 1) when no initialValue is provided, find the first present element (i.e. the first index k such that k in this), use that element as the initial accumulator and start iterating at k + 1; 2) if no such element exists (array is empty or only holes), throw a TypeError. Please change the logic to scan for the first present index and only if none found throw a TypeError. See also the separate comment about throwing a TypeError (line 19).

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.js

  }

+  if (arguments.length < 2 && this.length === 0) {
+    throw new Error('Array can not be empty.');


Do not throw a generic Error here. The task (and native behavior) requires throwing a TypeError when reduce is called on an empty array without an initial value. Replace throw new Error('Array can not be empty.') with something like throw new TypeError('Reduce of empty array with no initial value') to match the required behavior.

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.js

+  if (arguments.length < 2 && this.length === 0) {
+    throw new Error('Array can not be empty.');
+  }
+


Iteration details: you should cache the array length at the start (e.g. const len = this.length) and iterate using that len so that elements added after reduce starts are not visited. Also, when iterating you must skip holes (only call callback for indices that exist — e.g. if (i in this) { prev = callback(...); }). Finally, add an explicit check near the top to verify callback is a function and throw a TypeError if not (this is required). The current loop (for (let i = startIndex; i < this.length; i++)) will be affected by mutations to this.length during iteration and will not skip sparse holes; please update to use a cached length and presence-checking.

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.test.js

+    expect(mockFn).toHaveBeenCalledWith(8, 7, 2, array);
+  });
+
+  it(`should return error,


Issue — incorrect expectation for empty-array-without-initialValue. The test currently expects a custom error message 'Array can not be empty.', but the task requires that the behavior match native Array.prototype.reduce, which throws a TypeError for an empty array when no initialValue is provided. Either update the test to expect a TypeError or change the implementation to throw a TypeError (prefer matching native behavior). See the implementation where a generic Error is thrown for this case and the task description requiring MDN-consistent behavior . (Adjust expectation to toThrow(TypeError).)

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.test.js

+  it(`should return error,
+     if array is empty and is lack of initial value`, () => {
+    expect(() => [].reduce2((a, c) =>
+      a + c)).toThrow('Array can not be empty.');


Line-level expectation uses the specific custom message. The assertion toThrow('Array can not be empty.') hard-codes the custom message and masks whether the implementation follows native semantics (which should be a TypeError). Prefer toThrow(TypeError) so the test verifies the type of error (matching native) rather than a bespoke message. If you intentionally want a custom message, update task justification — otherwise align with native behavior .

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.test.js

+    expect(() => array.reduce2(100)).toThrow(TypeError);
+  });
+
+  it(`should sum array with empty slots`, () => {


Sparse array test does not assert that holes are skipped. The test uses a callback that treats undefined as 0, so it will pass even if the implementation calls the callback for array holes (which is incorrect). To validate the spec (holes are skipped), use a jest.fn() callback and assert the expected number of calls and the indices visited — e.g., for [1, , 5] with an initialValue you should see calls for index 0 and 2 only (or assert calls length and specific toHaveBeenCalledWith index values). This will catch implementations that call the callback for missing elements.

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.test.js

+      arr + (curr !== undefined && curr !== null ? curr : 0), 0);
+
+    expect(result).toBe(6);
+  });


Missing test: empty array WITH initialValue should return the initialValue and must not call the callback. Add a test that does expect([].reduce2(mockFn, 'x')).toBe('x') and assert mockFn was not called. The current test suite lacks this required case from the task checklist (empty arrays with initialValue) and so does not fully validate native-like behavior .

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.test.js

+    expect(result).toBe(45);
+  });
+
+  it('should add values different from integer', () => {


Missing coverage for non-primitive accumulator types. You have a test for string concatenation (good), but the requirements also ask to verify behavior when reducing to arrays/objects (e.g., accumulating into an array, or merging objects). Add tests that reduce into an array and into an object to confirm accumulators of different types are handled correctly and that the final returned value equals the final accumulator.

mateacademy-ai-mentor · 2025-10-06T15:25:08Z

src/reduce.test.js

  });

-  // Add tests here
+  it('should pass index or array to callback as arguements', () => {


Missing tests for this/receiver and mutation-during-iteration behaviors. The description requires verifying that this is not implicitly bound and that array mutations during reduction follow native semantics (e.g., elements added after the current index are not visited, changes to elements not yet visited are observed). Add tests that: (a) call the callback in non-strict and strict contexts (or check this is undefined/global according to your environment) to ensure no implicit binding; (b) mutate the array during reduction (push new items after current index and mutate future items) and assert which elements were visited and the final result. These are missing in the current suite and are required by the task description .

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

Zibi95 · 2025-10-08T07:17:23Z

src/reduce.test.js

+    expect(callback).not.toHaveBeenCalled();
+    expect(callback).toHaveBeenCalledTimes(0);


To sprawdza jedno i to samo

Zibi95 · 2025-10-08T07:17:52Z

src/reduce.test.js

+    expect(result).toBe(10);
+  });
+
+  it('should work without initial array', () => {


Lepsza bylaby nazwa "should work with empty array"

Zibi95 · 2025-10-08T07:18:44Z

src/reduce.test.js

+    const result = nums.reduce2(callback);
+
+    expect(callback).toHaveBeenCalled();
+    expect(callback).toHaveBeenCalledTimes(3);


nums nie ma przypadkiem 4 itemów?

Zibi95 · 2025-10-08T07:20:57Z

src/reduce.test.js

+  it('should invoke callback for indexes without value', () => {
+    [1, '', '', 1, 2].reduce2(callback);
+
+    expect(callback).toHaveBeenCalledTimes(4);


a nie 5 razy?

Zibi95 · 2025-10-08T07:25:35Z

src/reduce.test.js

+  it('should skip initial value if it is specified', () => {
+    const result = [1, 1, 2].reduce2(callback, null);
+
+    expect(result).toBe(4);
+  });


To chyba nie sprawdza tego co zapisałeś. null przy operacjach matematycznych jest zmieniany na 0

array_method_reduce

d71159d

mateacademy-ai-mentor suggested changes Oct 6, 2025

View reviewed changes

Robert added 2 commits October 6, 2025 17:37

problem with loop

7e99aa4

problem with loop

f93fdcf

robsza1122 requested a review from mateacademy-ai-mentor October 6, 2025 15:44

mateacademy-ai-mentor reviewed Oct 6, 2025

View reviewed changes

new_solution

6077857

robsza1122 requested a review from mateacademy-ai-mentor October 6, 2025 18:25

Zibi95 suggested changes Oct 8, 2025

View reviewed changes

		expect(callback).not.toHaveBeenCalled();
		expect(callback).toHaveBeenCalledTimes(0);

Conversation

robsza1122 commented Oct 6, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants