Sheffield | 26-Jan-ITP | Martha Ogunbiyi| Sprint 1 | Coursework/sprint 1#1054
Sheffield | 26-Jan-ITP | Martha Ogunbiyi| Sprint 1 | Coursework/sprint 1#1054marthak1 wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
marthak1
added
the
Needs Review
| if (numericValues.length === 0) return null; | ||
|
|
||
| // Step 4: sort safely | ||
| const sortedList = numericValues.slice().sort((a, b) => a - b); |
There was a problem hiding this comment.
Why make a copy of numericValues before sorting?
| // Given an array with no duplicates | ||
| // When passed to the dedupe function | ||
| // Then it should return a copy of the original array | ||
| test("given an array with no duplicates, it returns return a copy of the original array", () => { | ||
| const input = [1, 2, 3, 4, 5]; | ||
| const result = dedupe(input); | ||
| expect(result).toEqual([1, 2, 3, 4, 5]); | ||
| }); |
There was a problem hiding this comment.
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.
Can you find out what this additional check is?
| // Given an array with non-number values | ||
| // When passed to the max function | ||
| // Then it should return the max and ignore non-numeric values | ||
| test("given an array with non-number value, it should return the max and ignore non-numeric value", () => { | ||
| const input = [4, 5, 'arr', 2, []," ", 15, 8]; | ||
| const result = findMax(input); | ||
| expect(result).toBe(15); | ||
| }); | ||
|
|
||
| // 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 value, it should return the least surprising value given how it behaves for all other inputs", () => { | ||
| const input = [true, {}, "arr", null, [], " ", NaN]; | ||
| const result = findMax(input); | ||
| expect(result).toBe(-Infinity); | ||
| }); No newline at end of file |
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.
| // Given an array with decimal/float numbers | ||
| // When passed to the sum function | ||
| // Then it should return the correct total sum | ||
| test("given an array with decimal/float numbers, it should return the correct total sum", () => { | ||
| const input = [5, 6.6, 10, 5.12]; | ||
| const result = sum(input); | ||
| expect(result).toBe(26.720000000000002); | ||
| }); |
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
|
|
||
| // const array = ["a", "b", "c"]; | ||
|
|
||
| // for (const element of array) { | ||
| // console.log(element); | ||
| // } for (variable of iterable) statement | ||
|
|
There was a problem hiding this comment.
It's best practice to remove unnecessary code to keep the code clean.
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
implemented sprint 1 course work