London | 26-ITP-Jan | Zadri Abdule | Sprint 1 | Data - Groups#1090
London | 26-ITP-Jan | Zadri Abdule | Sprint 1 | Data - Groups#1090Zadri415 wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
Zadri415
added
Needs Review
| test("array with no duplicates returns a copy", () => { | ||
| const arr = [1, 2, 3]; | ||
| const out = dedupe(arr); | ||
| expect(out).toEqual(arr); | ||
| expect(out).not.toBe(arr); // should be a new array | ||
| }); |
There was a problem hiding this comment.
There is a chance that, even though out has incorrect elements (for example, []),
the two tests on lines 27-28 could still pass. Can you figure out why, and then fix the tests accordingly?
| test("given an array with non-number values, returns the max and ignores non-numeric values", () => { | ||
| expect(findMax(["hey", 10, "hi", 60, 10])).toBe(60); | ||
| }); | ||
|
|
||
| // 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(["hey", "hi", "there"])).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.
There was a problem hiding this comment.
Thanks. I'll add a test that includes a numeric-looking string("300") to ensure those strings are ignored, and implement findMax to only consider real number values.
| for (const el of elements) { | ||
| if (typeof el === "number") { | ||
| total += el; | ||
| } | ||
| } |
There was a problem hiding this comment.
What do you expect from the following function calls (on extreme cases)?
Does your function return the value you expected?
sum([NaN, 1]);
sum([Infinity, -Infinity]);
There was a problem hiding this comment.
Thanks - great question. I chose to ignore non finite values so the function only adds finite numbers. That means:
sum[NaN, 1] -> 1
sum [Infinity, -Infinity]) -> 0
| test("given an array with decimal numbers, returns the correct sum", () => { | ||
| expect(sum([1.5, 2.5, 3.5])).toBe(7.5); | ||
| }); |
There was a problem hiding this comment.
Decimal numbers in most programming languages (including JS) are internally represented in "floating point number" format. Floating point arithmetic is not exact. For example, the result of 46.5678 - 46 === 0.5678 is false because 46.5678 - 46 only yield a value that is very close to 0.5678. Even changing the order in which the program add/subtract numbers can yield different values.
So the following could happen
expect( 1.2 + 0.6 + 0.005 ).toEqual( 1.805 ); // This fail
expect( 1.2 + 0.6 + 0.005 ).toEqual( 1.8049999999999997 ); // This pass
expect( 0.005 + 0.6 + 1.2 ).toEqual( 1.8049999999999997 ); // This fail
console.log(1.2 + 0.6 + 0.005 == 1.805); // false
console.log(1.2 + 0.6 + 0.005 == 0.005 + 0.6 + 1.2); // falseCan you find a more appropriate way to test a value (that involves decimal number calculations) for equality?
Suggestion: Look up
- Checking equality in floating point arithmetic in JavaScript
- Checking equality in floating point arithmetic with Jest
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
Questions
N/A