Manchester | 26-ITP-Jan | Ofonime Edak | Sprint 1 | Data group#1045
Manchester | 26-ITP-Jan | Ofonime Edak | Sprint 1 | Data group#1045Ofonimeedak wants to merge 10 commits intoCodeYourFuture:mainfrom
Conversation
Ofonimeedak
added
📅 Sprint 1
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Why not practice "committing files one by one, on purpose, and for a reason"?
In VSCode, you can select which file to stage, and commit only the staged file.
See: https://www.youtube.com/watch?v=z5jZ9lrSpqk&t=705 (At around 12:50 minute marker, the video shows how to stage a single file).
| [{ input: [0.5, 0.2, 0.11, 0.89, 0.3], expected: 2 }].forEach( | ||
| ({ input, expected }) => | ||
| it(`should return ${expected} for [${input}]`, () => { | ||
| expect(sum(input)).toEqual(expected); | ||
| }) | ||
| ); |
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
There was a problem hiding this comment.
Thank you for this. I have read about the matchers for numbers and floats now
There was a problem hiding this comment.
I don't see any change made to the relevant Jest test.
Sprint-1/implement/sum.test.js
Outdated
| // Then it should ignore the non-numerical values and return the sum of the numerical elements | ||
|
|
||
| [ | ||
| { input: ["evan", 3, "mike", 20, 6, "", "/", , , 20], expected: 49 }, |
There was a problem hiding this comment.
Better to explicitly specify undefined instead of leaving the element blank.
cjyuan
added
Reviewed
Ofonimeedak
added
the
Needs Review
cjyuan
removed
the
Needs Review
Ofonimeedak
added
the
Needs Review
cjyuan
removed
the
Needs Review
Ofonimeedak
added
the
Needs Review
cjyuan
removed
the
Needs Review
Ofonimeedak
added
the
Needs Review
Sprint-1/implement/dedupe.test.js
Outdated
| expect(dedupe(input)).toStrictEqual(expected); | ||
| expect(dedupe(input)).not.toBe(expected); |
There was a problem hiding this comment.
What is the purpose of the test on line 31? It will always pass.
cjyuan
removed
the
Needs Review
Ofonimeedak
added
the
Needs Review
cjyuan
added
Complete
|
Looks good. |
|
Thank you, Sir. It's been a long journey....... I am learning well |
2 participants
Self checklist
Changelist
I have made changes to the following files, sum.js,include.js and all test cases
I have followed the acceptance criteria for all katas
This PR contains functions for de-duplication and summation of arrays
It also contained a refactored function and different test cases