Repository navigation
London | 26-ITP-Sep | Chandramani Gaire | Sprint 1 | Module-Data-Group - #1495
gaireprakash20-ops wants to merge 12 commits into
Conversation
… when calculating the median
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
gaireprakash20-ops
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
gaireprakash20-ops
changed the title
gaireprakash20-ops
added
the
Needs Review
cjyuan
added
Review in progress
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
-
Function implementations are all correct. Well done.
-
Regarding the PR description, could you use AI to research what should be included in the "Changelist" section of a PR, then revise your Changelist content accordingly?
-
You missed updating a file in the
refactorsub-folder.
| // Given an array with no duplicates | ||
| // When passed to the dedupe function | ||
| // Then it should return a copy of the original array | ||
| it("returns a copy of the array when there are no duplicates", () => { | ||
| // const list = [1, 2, 3]; | ||
| // const result = dedupe(list); | ||
| // expect(result).toEqual([1, 2, 3]); | ||
| // expect(result).not.toBe(list); | ||
| expect(dedupe([1, "1", 2])).toEqual([1, "1", 2]); | ||
| }); |
There was a problem hiding this comment.
Your function implementation is correct. However, this test could be improved to better ensure
that any future changes continue to align with the expected behavior described on line 25:
Then it should return a copy of the original array
This test should fail if the function returns the original array (instead of a copy of the original array).
The current test checks only if both the original array and the returned array contain identical elements.
In order to validate the returned array is a different array, we need an additional check.
Could you find out what this additional check is?
Also, any unused code should be deleted instead of kept in comment.
There was a problem hiding this comment.
What should calculateMean([Infinity, 0, -Infinity]) return, or how should the function behave?
The same also applies to sum().
| let Total = 0; | ||
| for (const value of elements) { | ||
| Total += value; |
There was a problem hiding this comment.
Some of the variable names does not quite match JS naming convention.
cjyuan
added
Reviewed
Learners, PR Template
Self checklist
Task code
CYF-1071
Changelist
i test and implement different function.