SolV1#1017
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation of splitInteger is correct and meets the core requirements: it returns exactly numberOfParts integers, sums to value, is sorted ascending, and ensures max−min ≤ 1. The main test suite inside the describe block effectively checks the key behaviors and should pass with your solution.
Minor improvements to consider:
- src/splitInteger.test.js: Remove or fill the empty placeholder tests at lines 6, 9, 11, and 13. They add noise without coverage. If you keep them, add concrete assertions or merge with the existing
describetests. - Add an explicit test for the case when
value < numberOfParts, e.g.,expect(splitInteger(3, 5)).toEqual([0, 0, 1, 1, 1]), to lock in that important requirement. - .eslintrc.js: Prefer
'no-proto': 'off'over numeric0and add a brief comment if disabling is intentional for clarity.
Nice job on a clean, concise algorithm and clear tests overall. These tweaks will make the suite more complete and maintainable.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| 'no-proto': 0, | ||
| strict: 0, | ||
| 'max-len': 0, | ||
| 'no-proto': 0 |
There was a problem hiding this comment.
This line disables the no-proto rule using the numeric severity 0. For readability prefer the string form: "no-proto": "off". Also ensure disabling this rule is intentional (add a short comment if so) so future maintainers know why __proto__ usage is allowed.
| if a value is divisible by a numberOfParts`, () => { | ||
|
|
||
| }); | ||
| if a value is divisible by a numberOfParts`, () => {}); |
There was a problem hiding this comment.
This test is an empty placeholder. Either remove it or provide assertions. The intended check (split into equal parts when divisible) is already covered inside the describe block, so consider removing this duplicate placeholder to avoid clutter.
|
|
||
| test(`should return a part equals to a value | ||
| when splitting into 1 part`, () => { | ||
| when splitting into 1 part`, () => {}); |
There was a problem hiding this comment.
Empty placeholder for "splitting into 1 part". Fill it with assertions (for example: expect(splitInteger(8, 1)).toEqual([8])) or remove it; the describe block already contains these assertions.
| when splitting into 1 part`, () => {}); | ||
|
|
||
| }); | ||
| test("should sort parts ascending if they are not equal", () => {}); |
There was a problem hiding this comment.
Empty placeholder for sorting behavior. The suite already has an explicit test that asserts the returned array is sorted ascending (lines 45-60). Either implement this here with distinct cases or remove to avoid duplication.
| test("should sort parts ascending if they are not equal", () => {}); | ||
|
|
||
| test('should sort parts ascending if they are not equal', () => { | ||
| test("should add zeros if value < numberOfParts", () => {}); |
There was a problem hiding this comment.
Empty placeholder for the case when value < numberOfParts. This important case is not covered elsewhere. Add a concrete assertion such as:
expect(splitInteger(3, 5)).toEqual([0, 0, 1, 1, 1]);
This will verify zeros are added and the result stays sorted as required by the task description.
No description provided.