-
-
Notifications
You must be signed in to change notification settings - Fork 222
London | ITP-Jan-26 | Mohsen Zamani | Sprint 1 | Coursework #949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
London | ITP-Jan-26 | Mohsen Zamani | Sprint 1 | Coursework #949
Conversation
Sprint-1/implement/dedupe.test.js
Outdated
| }, | ||
| ].forEach(({ input, expected }) => | ||
| it("returns a copy of original array when passed array with no duplicates ", () => | ||
| expect(dedupe(input)).toEqual(expected)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 implement this additional check?
Sprint-1/implement/sum.test.js
Outdated
| it("returns the correct sum for array containing decimal/float numbers", () => | ||
| expect(sum(input)).toBe(expected)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was interesting. Thank you.
And hope I got it right.
You can delete the original comments if the same information can be found in the test description.
I think ChatGPT can probably give you a more complete suggestion. |
cjyuan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good.
Just the part that checks "if the function returns a copy of the original array" is not quite correct. Can you fix it?
| it( | ||
| "returns a copy of original array when passed array with no duplicates ", | ||
| () => expect(dedupe(input)).toEqual(expected), | ||
| expect(dedupe(input)).not.toBe(expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 33 does not quite check if the function returns a copy of the original array.
| { input: [-7958463, -100, -202, -6453], expected: -7965218 }, | ||
| ].forEach(({ input, expected }) => | ||
| it("returns the correct sum for array with only negative values", () => | ||
| expect(sum(input)).toBeCloseTo(expected)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the input does not involve decimal numbers, toEqual() is better.
All integers in the interval [-Number.MAX_SAFE_INTEGER, Number.MAX_SAFE_INTEGER] can be represented precisely.
Learners, PR Template
Self checklist
Changelist
Completed tasks of Sprint 1: fix, implement, refactor, stretch folders
Questions