Solution#681
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the tests in src/getCoinCombination.test.js do not yet fully satisfy the explicit task requirements. The description calls for dedicated tests for the exact example outputs getCoinCombination(1) === [1, 0, 0, 0] and getCoinCombination(6) === [1, 1, 0, 0] (along with 17 and 50), and currently only 17 and 50 are directly covered. This means checklist items [CHECKLIST ITEM #7] and [CHECKLIST ITEM #8] are not met, which is a blocker for completing the task as specified.
Critical issues to fix:
- Add an
itblock that callsgetCoinCombination(1)and asserts that it returns[1, 0, 0, 0]using a strict equality check liketoEqual. - Add a separate
itblock that callsgetCoinCombination(6)and asserts that it returns[1, 1, 0, 0].
You’ve done a good job already covering other behaviors and ranges, so once you add these explicit example tests, your solution should fully meet the checklist requirements. Keep going—you’re very close to the finish line.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
|
|
||
| it(`'cents' is an exact multiple of 25 — only quarters used`, () => { | ||
| expect(getCoinCombination(50)).toEqual([0, 0, 0, 2]); | ||
| }); |
There was a problem hiding this comment.
The task description requires explicit tests for the example behaviors, including getCoinCombination(1) === [1, 0, 0, 0] and getCoinCombination(6) === [1, 1, 0, 0]; these specific cases are currently missing, so consider adding dedicated it blocks for cents 1 and 6 to satisfy checklist items #7 and #8.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because the test suite now fully meets the task requirements and correctly exercises getCoinCombination for all specified example inputs. You have clear Jest tests for 1, 6, 17, and 50 cents, and each expectation matches the required output format [pennies, nickels, dimes, quarters], confirming the interpretation of the returned array. The tests focus only on the src/getCoinCombination.js implementation without modifying it, which respects the constraints of the assignment. You also included a basic sanity check to ensure the function is declared and returns an array, which adds a small but useful layer of robustness.
There are no critical functional gaps or blockers remaining: all checklist items, including the dedicated tests for 1 cent [CHECKLIST ITEM #7] and 6 cents [CHECKLIST ITEM #8], are satisfied. As an optional improvement, you could add one or two more mixed-coin cases (for example, 23 or 41 cents) to further validate combinations beyond the given examples, but this is not required for completion. Overall, this is a solid, focused test file that aligns well with the assignment’s intent—nice work bringing it to completion.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.