Skip to content

Solution#1049

Open
hannahro15 wants to merge 3 commits intomate-academy:mainfrom
hannahro15:hannah-branch
Open

Solution#1049
hannahro15 wants to merge 3 commits intomate-academy:mainfrom
hannahro15:hannah-branch

Conversation

@hannahro15
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because one critical requirement from the task description is not yet met: the test suite does not include the four explicitly required example cases. The description states that you must assert the outputs for splitInteger(8, 1), splitInteger(6, 2), splitInteger(17, 4), and splitInteger(32, 6), but src/splitInteger.test.js currently omits these checks, so checklist items #4#7 are not satisfied. To fix this, add tests that assert the exact arrays:

  • splitInteger(8, 1) === [8]
  • splitInteger(6, 2) === [3, 3]
  • splitInteger(17, 4) === [4, 4, 4, 5]
  • splitInteger(32, 6) === [5, 5, 5, 5, 6, 6]
    You can either add four separate test(...) blocks following your existing style, or a single parameterized/table-driven test that iterates over these cases. Your implementation itself looks solid and your existing tests cover several important behaviors, so once these required example assertions are added, the solution should fully meet the task requirements.

✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example


test(`should split a number into equal parts
if a value is divisible by a numberOfParts`, () => {
const value = 12;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test suite is missing the required example test cases from the task description. Please add tests asserting:

  • splitInteger(8, 1) === [8]
  • splitInteger(6, 2) === [3, 3]
  • splitInteger(17, 4) === [4, 4, 4, 5]
  • splitInteger(32, 6) === [5, 5, 5, 5, 6, 6]
    You can follow the existing test patterns and add four new test(...) blocks (or combine some into one table-driven test). This is required to satisfy checklist items Solution #4split int test solution #7 from the description.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because one core requirement from the task description is still not fully met in src/splitInteger.test.js. The function implementation looks correct and you already assert the required example splitInteger(8, 1) === [8], but the tests are missing the three other explicitly required example checks. Specifically, you must add assertions for:

  • expect(splitInteger(6, 2)).toEqual([3, 3]);
  • expect(splitInteger(17, 4)).toEqual([4, 4, 4, 5]);
  • expect(splitInteger(32, 6)).toEqual([5, 5, 5, 5, 6, 6]);

Add these in src/splitInteger.test.js (for example, near your other divisible and non-divisible cases) either as separate test(...) blocks or as a single parameterized test; the style is flexible, but the exact arrays and inputs must be present as tests. Once these three assertions are in place, your test suite will directly reflect all examples required by the task description and the solution will be ready to pass. Your existing tests already cover behavior like sorting and near-even splits nicely, so after this adjustment your coverage will be both correct and complete.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

test(`should split a number into equal parts
if a value is divisible by a numberOfParts`, () => {

expect(splitInteger(8, 1)).toEqual([8]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good: you already assert splitInteger(8, 1) === [8] here (one of the required examples). You still need to add expect(splitInteger(6, 2)).toEqual([3, 3]) — consider adding it near these divisible-by cases so the examples are grouped together.

expect(splitInteger(8, 1)).toEqual([8]);
expect(splitInteger(8, 2)).toEqual([4, 4]);
expect(splitInteger(9, 3)).toEqual([3, 3, 3]);
expect(splitInteger(20, 4)).toEqual([5, 5, 5, 5]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the two missing example assertions expect(splitInteger(17, 4)).toEqual([4, 4, 4, 5]) and expect(splitInteger(32, 6)).toEqual([5, 5, 5, 5, 6, 6]). You can put them in this test block or create separate test(...) blocks; the description requires these exact checks.


test('should add zeros if value < numberOfParts', () => {

expect(splitInteger(4, 8)).toEqual([0, 0, 0, 0, 1, 1, 1, 1]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style note: either add four separate test(...) blocks or a single parameterized/table-driven test that iterates the four required cases — both approaches are acceptable as long as the exact arrays are asserted.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution as an educational override so you can continue progressing while you practice refining your testing style. Your splitInteger implementation and its tests correctly satisfy the core requirements: the examples like splitInteger(8, 1), splitInteger(6, 2), splitInteger(17, 4), and splitInteger(32, 6) produce the expected results, the output is sorted, all parts are integers, and the max–min difference is at most 1. One area to improve is test organization: instead of combining multiple behaviors in a single test, you could use separate describe blocks (for divisible, non-divisible, and zero-padding cases) so that failures are easier to interpret. As a further enhancement, you might add a couple of edge-case tests (for example, very large integers or 0 split into several parts) to build confidence that your logic holds for all valid inputs. Overall, you’ve met the task’s functional requirements well—these refinements will mainly help with readability and maintainability as you work on larger projects.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants