Sheffield | 26-ITP-jan | Richard Frimpong | Sprint 1 | Data Groups#1032
Sheffield | 26-ITP-jan | Richard Frimpong | Sprint 1 | Data Groups#1032Richiealx wants to merge 9 commits intoCodeYourFuture:mainfrom
Conversation
cjyuan
left a comment
There was a problem hiding this comment.
Code is good. I only have a few suggestions.
| <<<<<<< HEAD | ||
| // Keep only real numeric values | ||
| const numbersOnly = list.filter( | ||
| (item) => typeof item === "number" && !Number.isNaN(item) | ||
| ); | ||
|
|
||
| // Return null if the array contains no numbers | ||
| ======= | ||
| // filter() returns a new array, so this does not modify the original input | ||
| const numbersOnly = list.filter((item) => Number.isFinite(item)); | ||
|
|
||
| // Return null if there are no numeric values | ||
| >>>>>>> a22ed15 (Address mentor feedback for sprint 1 data groups) |
There was a problem hiding this comment.
This is a unresolved merge conflict. You need to delete the "merge conflict markers" and the code that you do not want to keep.
There are several others on the files in this branch.
Can you fix them?
There was a problem hiding this comment.
Thank you for pointing that out.
I have now resolved all merge conflicts by removing the conflict markers and keeping the correct final versions of the code.
I verified that no conflict markers remain, re-ran all Sprint 1 tests, and confirmed everything is passing before pushing the updated changes.
| test("given an array with no duplicates, it returns a copy of the original array", () => { | ||
| const input = [1, 2, 3]; | ||
| const result = dedupe(input); | ||
|
|
||
| // Given an array of strings or numbers | ||
| expect(result).toEqual(input); | ||
| expect(result).not.toBe(input); | ||
| }); |
There was a problem hiding this comment.
There is a chance both tests on lines 30-31 pass, but result and input have incorrect elements. Can you figure out why?
There was a problem hiding this comment.
Thank you. I see the issue now.
I updated the test so it no longer relies on comparing result with input alone. It now checks that:
- the returned array has the expected values
- the returned array is a different array object
- the original input array remains unchanged
I re-ran the implement tests and confirmed everything is passing.
|
Changes are all good. Well done. |
|
@cjyuan Thank you for your guidance! |
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |
Learners, PR Template
Self checklist
Changelist
This pull request contains my completed work for Sprint 1 - Data Groups.
Fix
Completed the fix exercise by correcting the implementation of:
Sprint-1/fix/median.jsChanges made:
nullwhen no numeric values are presentImplement
Completed the implement exercises by implementing the required functions in:
Sprint-1/implement/sum.jsSprint-1/implement/max.jsSprint-1/implement/dedupe.jsI also completed the related test files:
Sprint-1/implement/sum.test.jsSprint-1/implement/max.test.jsSprint-1/implement/dedupe.test.jsImplemented functionality includes:
Refactor
Completed the refactor exercise by updating:
Sprint-1/refactor/includes.jsChanges made:
forloop with afor...ofloopTesting
All tests were run locally using: