Skip to content

Conversation

@enjoy15
Copy link

@enjoy15 enjoy15 commented Oct 26, 2025

Learners, PR Template

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

-Completed all excercises in Sprint-1

@enjoy15 enjoy15 added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 26, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

You missed updating Sprint-1/implement/sum.js and Sprint-1/implement/sum.test.js.

Comment on lines +27 to +30
test("given an array with no duplicates, returns a copy of the original array", () => {
expect(dedupe([1, 2, 3])).toEqual([1, 2, 3]);
expect(dedupe(["a", "b", "c"])).toEqual(["a", "b", "c"]);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

The current test code cannot check if the returned array is a copy of the original array because toEqual() compares objects (including arrays) by value. To illustrate,

  const A = [2, 3, 1];
  const B = [...A];          // B is a copy of A
    
  // This set of code cannot distinguish if the compared objects are the same objects.
  expect(A).toEqual(A);  // true
  expect(A).toEqual(B);  // true

In order to check if the returned array is a copy of the original array, we would need additional checks.
Can you find out what code you need to add in order to ensure the returned value is not the original array?

Comment on lines +18 to +24
for (let i = 1; i < numbers.length; i++) {
if (numbers[i] > max) {
max = numbers[i];
}
}
return max;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you improve the indentation of this code?

Comment on lines +8 to +14
if (numbers.length === 0) {
if (elements.length === 0) {
return -Infinity;
} else {
return null;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also consider an array with no numbers the same as an array with no elements. The latter also does not have any number.

@cjyuan cjyuan 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. labels Nov 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 1 Assigned during Sprint 1 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants