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
57 changes: 51 additions & 6 deletions src/reduce.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,61 @@
* @returns {*}
*/
function reduce(callback, startValue) {
if (typeof callback !== 'function') {
throw new TypeError('Callback is not a function');
}
Comment on lines +10 to +12

Choose a reason for hiding this comment

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

Line 10–12: Good — the early typeof callback !== 'function' guard is present and correctly throws a TypeError. This satisfies the requirement that a non-function callback causes a TypeError (checklist 3.4).


const copyOfThis = { ...this };

Choose a reason for hiding this comment

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

Line 14: Critical issue — const copyOfThis = { ...this }; creates a snapshot copy of the object's indexed properties. Native reduce semantics require checking property presence on the original object at visit time (so deletions or modifications done by the callback before an index is visited affect iteration). Creating a shallow copy here prevents observing runtime mutations and will cause tests that mutate/delete elements during reduction to fail. Replace this with const O = Object(this); and iterate O directly using in checks at visit time. See tests that modify/delete/append array elements during callback execution for expected native-like behaviour.

const isArrayLike = this instanceof Object
&& this.hasOwnProperty('length')
&& typeof this.length === 'number'
&& !Array.isArray(this);
Comment on lines +15 to +18

Choose a reason for hiding this comment

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

Lines 15–18: The isArrayLike detection is fragile and unnecessary. Using Object(this) and const len = O.length >>> 0; plus visit-time in checks will correctly handle arrays and array-like objects. Removing the isArrayLike branching will simplify logic and better match required semantics (checklist 3.7).


Object.defineProperty(copyOfThis, 'length', {
value: this.length,
enumerable: false,
});

const len = copyOfThis.length >>> 0;
Comment on lines +20 to +25

Choose a reason for hiding this comment

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

Lines 20–25: You define length on the copied object and then use const len = copyOfThis.length >>> 0;. The numeric length snapshot must come from the original object (O.length >>> 0). Snapshot only the numeric length; do not pre-copy the indexed elements. This preserves the required snapshot-of-length but visit-time property checks for element presence.

const hasStartValue = arguments.length >= 2;

if (len < 1 && !hasStartValue) {
// eslint-disable-next-line max-len
throw new TypeError(`Array is empty and doesn't have 'startValue' argument`);
}
Comment on lines +28 to +31

Choose a reason for hiding this comment

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

Lines 28–31: The check if (len < 1 && !hasStartValue) throw TypeError is correct in principle, but ensure len was derived from Object(this).length >>> 0 (not from a copied object). Tests rely on this behaviour when an empty array has no initial value.


let prev = startValue;
let startIndex = 0;
let start = 0;

if (arguments.length < 2) {
startIndex = 1;
prev = this[0];
if (!hasStartValue) {
for (let i = 0; i < len; i++) {
if (i in copyOfThis) {
prev = copyOfThis[i];
start = i + 1;
break;
} else if (i === len) {
throw new TypeError(`Index hasn't been founded`);
}
}
Comment on lines +36 to +45

Choose a reason for hiding this comment

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

Lines 36–45: The no-initial-value branch scans copyOfThis to find the first present index. That scan must be performed on the original object (O) using if (i in O). Also, else if (i === len) is unreachable because the loop condition is i < len. Remove the unreachable else if (i === len) and ensure the search uses O so sparse arrays and array-likes are handled correctly (checklist 3.2, 3.6, 3.7).

}

for (let i = startIndex; i < this.length; i++) {
prev = callback(prev, this[i], i, this);
const copyOfThisArr = Object.assign([], copyOfThis);

Choose a reason for hiding this comment

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

Line 48: const copyOfThisArr = Object.assign([], copyOfThis); — this is another snapshot of indexed elements. It prevents observing modifications/deletions made to the original object during reduction and also means the callback will be passed a copy instead of the original object. The tests assert the callback receives the original array/object as its fourth argument and that runtime mutations influence iteration; using this snapshot will break those tests. Remove snapshotting and iterate O directly.


for (let i = start; i < len; i++) {
if (!isArrayLike) {
if (!(i in copyOfThisArr)) {
continue;
}

prev = callback(prev, copyOfThisArr[i], i, copyOfThisArr);
continue;
Comment on lines +50 to +57

Choose a reason for hiding this comment

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

Lines 50–57: In the main loop you use copyOfThisArr for non-array-like objects and call callback(..., copyOfThisArr). This passes a copy to the callback. The correct behaviour is to call callback(acc, O[i], i, O) where O is the original object. Also, visit-time presence must be checked on O (if (!(i in O)) continue;). Update this loop to iterate O with in checks and pass O as the 4th arg to match native semantics and the tests (checklist 3.9, 3.11).

}

if (!(i in copyOfThis)) {
continue;
}

prev = callback(prev, copyOfThis[i], i, copyOfThis);
Comment on lines +60 to +64

Choose a reason for hiding this comment

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

Lines 60–64: The array-like branch similarly checks i in copyOfThis and calls callback(..., copyOfThis). Again, pass the original object O as the 4th argument and perform presence checks on O at visit time. Unify handling instead of having separate branches — Object(this) + in checks covers both arrays and array-likes.

}

return prev;
Expand Down
Loading