Cape Town | 26-ITP-May| Enice Mutanda| Sprint 2| Data objects - #1419
Cape Town | 26-ITP-May| Enice Mutanda| Sprint 2| Data objects#1419Enice-Codes wants to merge 4 commits into
Conversation
Enice-Codes
added
Needs Review
Poonam-raj
added
Review in progress
| test("creates a country currency code lookup for multiple codes", () => { | ||
| const countryCurrencyPairs = [["US", "USD"], ["CA", "CAD"]]; | ||
| const result = createLookup(countryCurrencyPairs); | ||
| expect(result).toEqual({ US: "USD", CA: "CAD" }); | ||
| }); |
There was a problem hiding this comment.
I would consider having a smaller starting test for the valid input, to build the test suite up more gradually, prove the function can take different inputs and isn't hardcoded
| test("tally on an array with duplicate items returns counts for each unique item", () => { | ||
| expect(tally(["a"])).toEqual({ a: 1 }); | ||
| expect(tally(["a", "a", "a"])).toEqual({ a: 3 }); | ||
| expect(tally(["a", "a", "b", "c"])).toEqual({ a: 2, b: 1, c: 1 }); | ||
| }); |
There was a problem hiding this comment.
Repeated test as the below - remove and replace with the test described in the comment
| test("tally on an array with duplicate items returns counts for each unique item", () => { | ||
| expect(tally(["a"])).toEqual({ a: 1 }); | ||
| expect(tally(["a", "a", "a"])).toEqual({ a: 3 }); | ||
| expect(tally(["a", "a", "b", "c"])).toEqual({ a: 2, b: 1, c: 1 }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
I would argue these are three different behaviours - how might you split them up in the test suite?
| test("tally on an array with duplicate items returns counts for each unique item", () => { | ||
| expect(tally(["a"])).toEqual({ a: 1 }); | ||
| expect(tally(["a", "a", "a"])).toEqual({ a: 3 }); | ||
| expect(tally(["a", "a", "b", "c"])).toEqual({ a: 2, b: 1, c: 1 }); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
Again a repeated test - remove and replace with valid test for the given commented behaviour
There was a problem hiding this comment.
You're missing all the explanations and answers to questions a, b, c, c, d, e - please add them in
|
|
||
| test("calculates the total value of a till in pounds", () => { | ||
| const till = { "1p": 10, "5p": 6, "50p": 4, "20p": 10 }; | ||
| expect(totalTill(till)).toBe("£4.4"); | ||
| }); | ||
|
|
||
| test("returns £0 for an empty till", () => { | ||
| expect(totalTill({})).toBe("£0"); | ||
| }); |
There was a problem hiding this comment.
Strong case for building up the test suite incrementally instead of one test for the final behaviour because a lot of different elements build the complexity (different denominations of money, having multiple of one money, having multiple of multiple denominations)
| // a) What is the target output when totalTill is called with the till object | ||
|
|
||
| // b) Why do we need to use Object.entries inside the for...of loop in this function? | ||
|
|
||
| // c) What does coin * quantity evaluate to inside the for...of loop? | ||
|
|
||
| // d) Write a test for this function to check it works and then fix the implementation of totalTill |
|
|
||
| for (const [coin, quantity] of Object.entries(till)) { | ||
| total += coin * quantity; | ||
| const coinValue = parseInt(coin, 10); |
There was a problem hiding this comment.
nice use of parseInt to remove the p and convert to int
Poonam-raj
left a comment
Poonam-raj
left a comment
There was a problem hiding this comment.
Missing debug elements here - can you include it in this PR?
I have a few things I'd like you to take another look at, a few answers and explanations are missing. Some tests are also wrong (tally).
Some good logic choices, and methods used.
Poonam-raj
added
Reviewed
Enice-Codes
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
Enice-Codes
added
the
Needs Review
7c1b1ce to
bccf5d7
Compare
2 participants
Self checklist
Changelist
tested and passed all my code locally
modified the codes
added code to most dependent file
fix codes on certain exercises and implemented code too