Manchester | 26-ITP-Jan | Liban Jama | Sprint 1 | Data Groups #1016
Manchester | 26-ITP-Jan | Liban Jama | Sprint 1 | Data Groups #1016libanj0161 wants to merge 5 commits intoCodeYourFuture:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| let numbers = []; | ||
|
|
||
| // loop over each item in list. | ||
| for (const item of list) { | ||
| // if item is a number, push it into the numbers array. | ||
| if (typeof item === "number") { | ||
| numbers.push(item); | ||
| } | ||
| } |
There was a problem hiding this comment.
-
Could consider using array's
.filter()method to simplify the code on lines 15-23. -
Do you plan to consider
-Infinity,Infinity, andNaNin the median calculation (and also in the functions inimplement/max.jsandimplement/sum.js)?
| // Given an array with no duplicates | ||
| // When passed to the dedupe function | ||
| // Then it should return a copy of the original array | ||
|
|
||
| // Given an array with strings or numbers | ||
| // When passed to the dedupe function | ||
| // Then it should remove the duplicate values, preserving the first occurence of each element |
There was a problem hiding this comment.
There are two more tests to be implemented on this file.
| test("given an array with non-number values, returns the max and ignore non-numeric values", () => { | ||
| expect(findMax([3, "dogs", 1, "cat", null])).toBe(3); | ||
| }); | ||
|
|
||
| // Given an array with only non-number values | ||
| // When passed to the max function | ||
| // Then it should return the least surprising value given how it behaves for all other inputs | ||
|
|
||
| test("given an array with only non-number values, returns -Infinity", () => { | ||
| expect(findMax(["a", null, true, undefined])).toBe(-Infinity); | ||
| }); |
There was a problem hiding this comment.
When a string representing a valid numeric literal (for example, "300") is compared to a number,
JavaScript first converts the string into its numeric equivalent before performing the comparison.
As a result, the expression 20 < "300" evaluates to true.
To test if the function can correctly ignore non-numeric values,
consider including a string such as "300" in the relevant test cases.
Learners, PR Template
Self checklist
Changelist
Completed Fix under data groups module.
Questions
N/A.