WM | May-2025 | Abdullah Saleh | Module-Data-Groups | Sprint1 #561
Conversation
arun-john
added
Review in progress
3bdullah-saleh
added
Needs Review
| return null; | ||
| } | ||
| // Filter out non-numeric values (NaN, undefined, etc.) | ||
| const numbersOnly = list.filter(item => typeof item === 'number') |
There was a problem hiding this comment.
NaN is a value of type "number".
If you want to also filter out Infinity, consider using Number.isFinute().
| else if (array.length != 0) { | ||
| array.forEach(element => { | ||
| if (!arrayCopy.includes(element)) { | ||
| arrayCopy.push(element); | ||
| } | ||
| }); | ||
| return arrayCopy; | ||
| } | ||
|
|
There was a problem hiding this comment.
Indentation is off. Can you fix it?
| test.todo("Given an array with no duplicates, it return a copy of the original array"); | ||
| test("Given an array with no duplicates, it return a copy of the original array", () => { | ||
| expect(dedupe(["a", "b", "c"])).toEqual(["a", "b", "c"]); | ||
| }); |
There was a problem hiding this comment.
-
Can remove
test.todo(...)since you have implemented the test. -
The current test code cannot check if the returned array is a copy of the original array because
toEqual()compares objects (including arrays) by value. To illustrate,
const A = [2, 3, 1];
const B = [...A]; // B is a copy of A
// This set of code cannot distinguish if the compared objects are the same objects.
expect(A).toEqual(A); // true
expect(A).toEqual(B); // true
In order to check if the returned array is a copy of the original array, we would need additional checks.
Can you find out what code you need to add in order to ensure the returned value is not the original array?
| test("Given an array with only non-number values, should return Infinity", () => { | ||
| const input = ['hey', "10", 'hi', "60", "10"] | ||
| const output = Infinity | ||
| expect(findMax(input)).toEqual(output) | ||
| }); |
There was a problem hiding this comment.
-
Can your function pass this test?
-
Can you check if the following two function calls return the value you expect?
findMax(["A"]);
findMax([NaN, 1, 2, 3]);
| test("Given an array with just negative numbers, should return the closest number to zero", () => { | ||
| const input = [-1, -2, -1, -5, -15] | ||
| const output = 1 | ||
| expect(findMax(input)).toEqual(output) | ||
| }); | ||
|
|
There was a problem hiding this comment.
The largest number in input is -1, not 1.
| test("Given an array with decimal/float numbers, should return the correct total sum", () => { | ||
| const input = [1.5, 2.4, 3.5]; | ||
| const output = 7.4 | ||
| expect(sum(input)).toEqual(output) | ||
| }); | ||
|
|
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); // false
Can 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
| else if (elements.length > 1) { | ||
| const numbersOnly = elements.filter(element => typeof element === 'number'); | ||
| if (numbersOnly.length == 0) { | ||
| return Infinity; |
There was a problem hiding this comment.
What's your rationale to return Infinity when the array does not contain any number?
| test("Given an array with only non-number values, should return Infinity", () => { | ||
| const input = ["2.4", "hello", null, undefined]; | ||
| const output = Infinity | ||
| expect(sum(input)).toEqual(output) | ||
| }); |
There was a problem hiding this comment.
Can you check if your function returns the expected values in the following function calls?
sum(["A"]);
sum([-Infinity, 1, 2, 3]);
sum([-Infinity, Infinity]);
sum([Infinity, 1, 2, 3]);
cjyuan
added
Reviewed
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.