Conversation
Fix linter error in the task file
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the implementation is correct, but the test suite does not fully cover the explicit requirements from the task description, which is a functional requirement for this assignment. Your splitInteger function behaves as specified, but in src/splitInteger.test.js you are missing tests for the exact example cases: splitInteger(8, 1) -> [8], splitInteger(6, 2) -> [3, 3], splitInteger(17, 4) -> [4, 4, 4, 5], and splitInteger(32, 6) -> [5, 5, 5, 5, 6, 6] ([CHECKLIST ITEM #4–#7]). You also need an explicit property test that checks max - min <= 1 on a non-trivial result, for example:
const result = splitInteger(17, 4);
const diff = Math.max(...result) - Math.min(...result);
expect(diff).toBeLessThanOrEqual(1);Additionally, add an explicit assertion that the returned array length equals numberOfParts ([CHECKLIST ITEM #1]), e.g. expect(result).toHaveLength(numberOfParts) in at least one test (ideally in those new example tests). Once you add these tests, your suite will directly verify all required properties and example outputs, and your otherwise solid implementation will be fully backed by the required checks. Your core logic looks good, so this is mainly about strengthening and aligning the tests with the stated checklist; you’re very close to a complete solution.
✨ 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
|
|
||
| const result = splitInteger(1, 2) | ||
|
|
||
| expect(result).toEqual([0, 1]) |
There was a problem hiding this comment.
Add tests for the exact example cases required by the task description. The suite currently uses other values (10 and 1) but must include: splitInteger(8, 1) -> [8], splitInteger(6, 2) -> [3, 3], splitInteger(17, 4) -> [4, 4, 4, 5], and splitInteger(32, 6) -> [5, 5, 5, 5, 6, 6] (checklist items #4–#7). See the description for the exact examples .
src/splitInteger.test.js
Outdated
|
|
||
| const result = splitInteger(10, 3) | ||
|
|
||
| expect(result).toEqual([3, 3, 4]) |
There was a problem hiding this comment.
Add an explicit test that verifies the numeric property max - min <= 1 required by the task (checklist #2). For example: const diff = Math.max(...result) - Math.min(...result); expect(diff).toBeLessThanOrEqual(1) — add this for a non-trivial case (or for each example).
src/splitInteger.test.js
Outdated
|
|
||
| const result = splitInteger(10, 1) | ||
|
|
||
| expect(result).toEqual([10]) |
There was a problem hiding this comment.
Although some toEqual assertions implicitly verify length, add an explicit test asserting the returned array length equals numberOfParts (checklist #1). e.g. expect(result).toHaveLength(numberOfParts) — this makes the intent explicit in the test suite.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the test suite is still missing two explicit requirements from the task description, even though the core implementation of splitInteger is correct. First, there is no test for the required example splitInteger(17, 4) -> [4, 4, 4, 5]; this specific case must be asserted with expect(result).toEqual([4, 4, 4, 5]) and expect(result).toHaveLength(4) to fully demonstrate compliance. Second, none of the current tests asserts that the returned array length equals numberOfParts, which is a checklist requirement [CHECKLIST ITEM #1]; please add expect(result).toHaveLength(numberOfParts) in at least one test (for example, the new 17/4 test and/or the existing 6/2 and 32/6 tests). For clarity of the balancing property, it’s also a good idea (and was suggested) to add expect(Math.max(...result) - Math.min(...result)).toBeLessThanOrEqual(1) to the new 17/4 test so that both a simple and a more complex case explicitly verify the max–min constraint. Once you add the 17/4 test and at least one .toHaveLength(...) assertion, your solution will fully match the stated requirements and can be approved. Overall, the implementation itself is solid—these are just test coverage tweaks to make your solution complete and explicit about the required properties.
✨ 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
| const result = splitInteger(6, 2) | ||
|
|
||
| expect(result).toEqual([3, 3]) |
There was a problem hiding this comment.
Add an explicit length assertion for this test. For example, after const result = splitInteger(6, 2) add expect(result).toHaveLength(2). This satisfies the requirement that the returned array contains exactly numberOfParts elements (checklist item #1).
|
|
||
| test('should sort parts ascending if they are not equal', () => { | ||
|
|
||
| const result = splitInteger(32, 6) |
There was a problem hiding this comment.
The required example splitInteger(17, 4) -> [4, 4, 4, 5] is missing. Add a new test (before or after this test) that asserts the exact array, asserts .toHaveLength(4), and (optionally) checks Math.max(...result) - Math.min(...result) <= 1 so the property is explicit. Example:
test('example: splitInteger(17, 4) -> [4,4,4,5]', () => {
const result = splitInteger(17, 4)
expect(result).toEqual([4,4,4,5])
expect(result).toHaveLength(4)
expect(Math.max(...result) - Math.min(...result)).toBeLessThanOrEqual(1)
})
| expect(result).toEqual([5, 5, 5, 5, 6, 6]) | ||
| expect(diff).toBeLessThanOrEqual(1) |
There was a problem hiding this comment.
Consider adding a length assertion to this test as well (e.g. expect(result).toHaveLength(6)) so the suite demonstrates the length-check for a non-trivial case. At minimum, one .toHaveLength(...) must appear in the tests; adding it here makes the property explicit for the 32/6 example.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you learn, even though there is one missing test case to address. Your implementation of splitInteger is solid: it returns the correct number of parts, keeps them balanced (max–min ≤ 1), and outputs them in ascending order, which shows good understanding of the requirements.
The main issue is in src/splitInteger.test.js, where you haven’t yet added the required explicit test for splitInteger(17, 4) producing [4, 4, 4, 5]; this test should also check that the length is 4 and that Math.max(...result) - Math.min(...result) <= 1. To fix this, add a new Jest test block (as suggested in the review comment) right after the related tests, using expect(result).toEqual([4, 4, 4, 5]), expect(result).toHaveLength(4), and the balance assertion. As a minor improvement, you could also add explicit length checks to any tests that don’t have them yet for consistency.
Overall, your core logic and existing tests show strong progress—tightening the test suite with the 17/4 example will make your solution fully aligned with the specification. Keep up the good work; you’re very close to a thoroughly complete solution.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| expect(result).toEqual([5, 5, 5, 5, 6, 6]) | ||
| expect(result).toHaveLength(6) | ||
| expect(diff).toBeLessThanOrEqual(1) |
There was a problem hiding this comment.
Add a test for the required example splitInteger(17, 4) === [4, 4, 4, 5]. Place it after this test block. The test should assert the exact array, assert the length equals 4, and verify the balance constraint (max - min <= 1). Example to add:
test('should split 17 into 4 balanced parts', () => {
const result = splitInteger(17, 4)
expect(result).toEqual([4, 4, 4, 5])
expect(result).toHaveLength(4)
expect(Math.max(...result) - Math.min(...result)).toBeLessThanOrEqual(1)
})This addresses the missing required example from the description (HIGH priority).
No description provided.