Solution #438
Solution #438
Conversation
There was a problem hiding this comment.
Great job on implementing the getCoinCombination function and its tests! 🎉 The code meets the task requirements and functions as expected. While there are some minor improvements suggested for test case naming and code simplification, these do not impact the functionality. Keep up the great work and consider the feedback for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| if (isNaN(cents)) { | ||
| return coins; | ||
| } |
There was a problem hiding this comment.
The check for isNaN(cents) is redundant because the subsequent check for typeof cents !== 'number' will also handle non-numeric inputs. Consider removing the isNaN check to simplify the code.
| it(`should be positive number`, () => { | ||
| expect(getCoinCombination(0)).toEqual([0, 0, 0, 0]); | ||
| expect(getCoinCombination(6)).toEqual([1, 1, 0, 0]); | ||
| expect(getCoinCombination(17)).toEqual([2, 1, 1, 0]); | ||
| }); |
There was a problem hiding this comment.
The description 'should be positive number' is misleading because the test is checking the function's output for various valid inputs, not just positive numbers. Consider renaming it to something like 'should return correct coin combination for valid inputs'.
| it(`should be limit values`, () => { | ||
| expect(getCoinCombination(50)).toEqual([0, 0, 0, 2]); | ||
| expect(getCoinCombination(26)).toEqual([1, 0, 0, 1]); | ||
| expect(getCoinCombination(33)).toEqual([3, 1, 0, 1]); | ||
| }); |
There was a problem hiding this comment.
The description 'should be limit values' could be more descriptive. Consider renaming it to 'should handle edge cases correctly' to better reflect the purpose of these tests.
| it(`should be integer`, () => { | ||
| expect(getCoinCombination("1")).toEqual([0, 0, 0, 0]); | ||
| expect(getCoinCombination(0)).toEqual([0, 0, 0, 0]); | ||
| expect(getCoinCombination(NaN)).toEqual([0, 0, 0, 0]); | ||
| expect(getCoinCombination(null)).toEqual([0, 0, 0, 0]); | ||
| }); |
There was a problem hiding this comment.
The description 'should be integer' might be confusing as the test is checking how the function handles non-integer inputs. Consider renaming it to 'should handle non-integer inputs gracefully'.
Have a nice day!