Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 33 additions & 11 deletions src/reduce.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,46 @@

/**
* @param {function} callback
* @param {*} startValue
*
* @param {*} initialValue
* @returns {*}
*/
function reduce(callback, startValue) {
let prev = startValue;
let startIndex = 0;
function reduce(callback, initialValue) {
if (this == null) {
throw new TypeError('Array.prototype.reduce called on null or undefined');
Comment on lines +9 to +10

Choose a reason for hiding this comment

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

Good: guard for null/undefined receiver is present which ensures calling with this == null throws as required by the spec. This addresses the checklist item about throwing on null/undefined receivers.

}

if (typeof callback !== 'function') {
throw new TypeError(callback + ' is not a function');
Comment on lines +13 to +14

Choose a reason for hiding this comment

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

Good: the function checks the callback type and throws when it's not a function. This satisfies the checklist item that a non-function callback must cause a TypeError. The thrown message callback + ' is not a function' is acceptable, though you could prefer a message that does not coerce callback to string. Keep this check.

}
Comment on lines +14 to +15

Choose a reason for hiding this comment

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

Minor suggestion: the error message used when callback is not a function coerces the callback to string. It's fine, but you could use a more standard message such as 'Callback must be a function' or similar if you prefer predictable messages in tests — only change if your tests assert the exact message. The important part (throwing TypeError) is already correct.


const arr = Object(this);
const len = arr.length >>> 0;

Choose a reason for hiding this comment

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

Good: length is captured once into len (using >>> 0) so the iteration bounds are fixed and elements appended during iteration will not be visited. This resolves the earlier issue where dynamic arr.length could cause appended items to be visited.


let k = 0;
let accumulator;

if (arguments.length > 1) {
accumulator = initialValue;
} else {
// шукаємо перший існуючий елемент
while (k < len && !(k in arr)) {
k++;
}

if (k >= len) {
throw new TypeError('Reduce of empty array with no initial value');
}
Comment on lines +27 to +33

Choose a reason for hiding this comment

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

Good: the implementation explicitly scans for the first existing element when no initialValue is provided and throws TypeError('Reduce of empty array with no initial value') if none is found. This correctly covers the all-holes case.


if (arguments.length < 2) {
startIndex = 1;
prev = this[0];
accumulator = arr[k++];
}

for (let i = startIndex; i < this.length; i++) {
prev = callback(prev, this[i], i, this);
for (; k < len; k++) {

Choose a reason for hiding this comment

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

Good: the main for loop uses the captured len and checks k in arr to skip holes — this matches native reduce behavior for sparse arrays and ensures correct iteration counts.

if (k in arr) {
accumulator = callback(accumulator, arr[k], k, arr);

Choose a reason for hiding this comment

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

Minor suggestion: you call the callback as a bare function (callback(accumulator, arr[k], k, arr)), which is correct and will produce this === undefined in strict-mode callbacks. Consider adding a short comment explaining that this is intentional (so future readers understand this will be undefined for strict callbacks).

}
}

return prev;
return accumulator;
}
Comment on lines +44 to 45

Choose a reason for hiding this comment

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

Edge-case note: with the fixes above, ensure the behavior for single-element arrays is correct:

  • Without initialValue, return the single element without calling callback.
  • With initialValue, callback is called once.
    Your current structure supports these once the len and all-holes check are in place; just ensure tests reflect this.


module.exports = { reduce };
131 changes: 128 additions & 3 deletions src/reduce.test.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,144 @@
'use strict';

// eslint-disable-next-line no-extend-native
const { reduce } = require('./reduce');

describe('reduce', () => {
beforeAll(() => {
Array.prototype.reduce2 = reduce; // eslint-disable-line
// eslint-disable-next-line no-extend-native
Array.prototype.reduce2 = reduce;
});

afterAll(() => {
delete Array.prototype.reduce2;
});

it('should ', () => {
let callback;

beforeEach(() => {
callback = jest.fn().mockImplementation((a, b) => a + b);
});

// Add tests here
it('should be declared', () => {
expect(reduce).toBeInstanceOf(Function);
});

it('should not mutate array', () => {
const array = [1, 2, 3, 4];
const copy = [...array];
array.reduce2(callback, 0);
expect(array).toEqual(copy);
});

it('should run callback array`s length times if initialValue is provided', () => {
const array = [1, 2, 3, 4];

Check failure on line 34 in src/reduce.test.js

View workflow job for this annotation

GitHub Actions / build (20.x)

Line 34 exceeds the maximum line length of 80
array.reduce2(callback, 0);
expect(callback).toHaveBeenCalledTimes(array.length);
});

it('should run callback array`s length - 1 times if no initialValue', () => {
const array = [1, 2, 3, 4];
array.reduce2(callback);
expect(callback).toHaveBeenCalledTimes(array.length - 1);
Comment on lines +33 to +42

Choose a reason for hiding this comment

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

Suggestion: callback-count tests (should run callback array's length times... and ...length - 1 times if no initialValue) are correct for dense arrays, but add additional tests that assert call counts when the array is sparse (holes) to confirm holes are skipped and call counts reflect present elements only.

});

it('should not run callback if array is empty and initialValue is provided', () => {
const array = [];
array.reduce2(callback, 0);
expect(callback).toHaveBeenCalledTimes(0);

Check failure on line 48 in src/reduce.test.js

View workflow job for this annotation

GitHub Actions / build (20.x)

Line 48 exceeds the maximum line length of 80
});

it('should run callback with correct arguments when initialValue is provided', () => {
const array = [1, 2, 3, 4];
array.reduce2(callback, 0);

expect(callback).toHaveBeenNthCalledWith(1, 0, 1, 0, array);

Check failure on line 55 in src/reduce.test.js

View workflow job for this annotation

GitHub Actions / build (20.x)

Line 55 exceeds the maximum line length of 80
expect(callback).toHaveBeenNthCalledWith(2, 1, 2, 1, array);
expect(callback).toHaveBeenNthCalledWith(3, 3, 3, 2, array);
expect(callback).toHaveBeenNthCalledWith(4, 6, 4, 3, array);
});

Choose a reason for hiding this comment

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

Add an explicit test verifying starting-index behavior for the no initialValue path. The file already checks callback arguments with an initialValue, but the spec also requires verifying the other path: when no initialValue is provided the first callback invocation should have accumulator === array[0] and currentIndex === 1. Example you can add right after the initial-arguments test:

it('should start at index 1 and use array[0] as accumulator when no initialValue', () => {
  const array = [10, 20, 30];
  const cb = jest.fn((acc, cur, idx, arr) => acc + cur);
  array.reduce2(cb);
  expect(cb).toHaveBeenNthCalledWith(1, 10, 20, 1, array);
});

This ensures the starting-index behavior required by the spec is explicitly asserted.

it('should return initial value if array is empty', () => {
const array = [];
const initialValue = 0;
const result = array.reduce2(callback, initialValue);

expect(result).toBe(initialValue);
});

it('should throw if array is empty and no initial value', () => {
const array = [];
expect(() => array.reduce2(callback)).toThrow(TypeError);
});

Choose a reason for hiding this comment

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

Add a test for the single-element array without an initialValue. Per the spec, reduce should return that single element and should NOT call the callback. I don't see this specific assertion in the suite. Example to add near the empty-array/all-holes tests:

it('should return single element without calling callback when no initialValue', () => {
  const array = [42];
  const cb = jest.fn();
  const result = array.reduce2(cb);
  expect(result).toBe(42);
  expect(cb).toHaveBeenCalledTimes(0);
});

This closes the checklist item for single-element arrays.

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

Choose a reason for hiding this comment

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

Add a test covering arrays that include falsy values (0, '', false, null, undefined) to ensure they are processed when present (they should not be treated as holes). A minimal test could assert sum/concatenation/processing of those values to match native reduce. Example:

it('should process falsy values present in array', () => {
  const array = [0, '', false, null, undefined];
  const result = array.reduce2((acc, cur) => acc + String(cur) + '|', '');
  const expected = array.reduce((acc, cur) => acc + String(cur) + '|', '');
  expect(result).toBe(expected);
});

This verifies falsy but present elements are visited and passed to the callback.

it('should handle leading holes without initialValue', () => {
// eslint-disable-next-line no-sparse-arrays
const array = [, , 3, 4];
const result = array.reduce2((acc, cur) => acc + cur);
expect(result).toBe(7); // accumulator = 3, then adds 4
});

it('should throw on all-holes array without initialValue', () => {
const array = new Array(3); // [ , , , ]
expect(() => array.reduce2(callback)).toThrow(TypeError);
});

it('should throw if callback is not a function', () => {
const array = [1, 2];
expect(() => array.reduce2(null)).toThrow(TypeError);
});

it('should have undefined as callback this in strict mode', () => {
const array = [1];
let recordedThis;
array.reduce2(function (acc, cur) {
recordedThis = this;
return acc + cur;
}, 0);
expect(recordedThis).toBeUndefined();
});

it('should support array-like objects via call', () => {
const obj = { 0: 1, 1: 2, length: 2 };
const result = reduce.call(obj, (acc, cur) => acc + cur, 0);
expect(result).toBe(3);
});

it('should throw if called on null/undefined', () => {
expect(() => reduce.call(null, callback, 0)).toThrow(TypeError);
expect(() => reduce.call(undefined, callback, 0)).toThrow(TypeError);
});

it('should ignore non-numeric properties', () => {
const array = [1, 2, 3];
// @ts-ignore
array.foo = 10;
// element beyond initial length
array[100] = 100;

const result = array.reduce2((acc, cur) => acc + cur, 0);
expect(result).toBe(6); // only 1+2+3
});

Choose a reason for hiding this comment

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

Please add tests for: (1) verifying elements appended during iteration are ignored, and (2) a large-array deterministic test to ensure iteration completes and the callback is invoked the expected number of times.

Example for appended-element behavior:

it('should ignore elements appended after iteration begins', () => {
  const array = [1, 2, 3];
  const cb = jest.fn((acc, cur, idx, arr) => {
    if (idx === 0) arr.push(100); // appended during iteration
    return acc + cur;
  });
  const result = array.reduce2(cb, 0);
  expect(result).toBe(6); // 1+2+3, appended 100 must be ignored
  expect(cb).toHaveBeenCalledTimes(3);
});

Example for large-array test:

it('should iterate over large arrays and call callback expected number of times', () => {
  const size = 1000;
  const array = Array.from({ length: size }, (_, i) => i + 1);
  const cb = jest.fn((acc, cur) => acc + cur);
  const result = array.reduce2(cb, 0);
  expect(cb).toHaveBeenCalledTimes(size);
  expect(result).toBe(array.reduce((a, b) => a + b, 0));
});

These catch regressions where the implementation might read arr.length dynamically or otherwise visit appended items.

it('should work with string concatenation', () => {

Check failure on line 130 in src/reduce.test.js

View workflow job for this annotation

GitHub Actions / build (20.x)

unnecessary '.call()'
const array = ['a', 'b', 'c'];

Check failure on line 131 in src/reduce.test.js

View workflow job for this annotation

GitHub Actions / build (20.x)

unnecessary '.call()'
const result = array.reduce2((acc, cur) => acc + cur, '');
expect(result).toBe('abc');
});

Choose a reason for hiding this comment

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

Add a test ensuring the implementation matches native reduce when the accumulator changes type during reduction. The checklist requires correct behavior when the callback returns different types and accumulator changes type. Example:

it('should handle accumulator changing type during reduction', () => {
  const array = [1, 2, 3];
  const resultCustom = array.reduce2((acc, cur, i) => {
    if (i === 1) return String(acc) + cur; // change type to string
    return acc + cur;
  }, 0);
  const resultNative = array.reduce((acc, cur, i) => {
    if (i === 1) return String(acc) + cur;
    return acc + cur;
  }, 0);
  expect(resultCustom).toBe(resultNative);
});

This ensures type transitions of the accumulator behave identically to the native implementation.

it('should work with objects as accumulator', () => {
const array = ['x', 'y'];
const result = array.reduce2((acc, cur, i) => {
acc[i] = cur;
return acc;
}, {});
expect(result).toEqual({ 0: 'x', 1: 'y' });
});
});
Loading