js_split_integer by shelyaa · Pull Request #1018 · mate-academy/js_split_integer

shelyaa · 2025-09-18T21:57:27Z

No description provided.

mateacademy-ai-mentor

Review Summary

Decision: Request Changes

Your implementation of splitInteger looks correct and the existing example-based tests are clear. However, the task explicitly requires tests to assert the general properties of the result: length equals numberOfParts, all elements are integers, the sum equals value, the array is sorted ascending, and max - min ≤ 1. These property assertions are currently missing, which means the suite doesn’t fully verify the core requirements.

Critical issues to fix (blockers):

In src/splitInteger.test.js add the property assertions after computing result for each test case. For example (adjusting value and numberOfParts per test):
- expect(result).toHaveLength(numberOfParts)
- expect(result.every(Number.isInteger)).toBe(true)
- expect(result.reduce((a, b) => a + b, 0)).toBe(value)
- expect(result).toEqual(result.slice().sort((a, b) => a - b))
- expect(Math.max(...result) - Math.min(...result)).toBeLessThanOrEqual(1)
Add the missing documented example test: splitInteger(32, 6) and assert both the exact example [5,5,5,5,6,6] and the properties above.

Minor improvement (non-blocker):

In .eslintrc.js, prefer 'no-proto': 'off' over 0 for readability.

Once you add the property-based checks and the missing example, the tests will fully satisfy the checklist. Nice work on the implementation—this is a small but important enhancement to your test coverage.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-09-18T22:02:29Z

-    'no-proto': 0,
-    strict: 0,
-    'max-len': 0,
+    'no-proto': 0


The rule override disables no-proto using a numeric severity (0). This is valid, but for readability and explicitness prefer the string form. For example:

'no-proto': 'off'

Using 'off' makes the intent clearer to readers who may not remember numeric severities.

mateacademy-ai-mentor · 2025-09-18T22:02:29Z


 test(`should split a number into equal parts
  if a value is divisible by a numberOfParts`, () => {
+  const result = splitInteger(6, 2);


This line assigns the call result for the test splitting 6 into 2 parts. After capturing result you should add explicit assertions for the checklist properties (length, integer elements, sum, sorted ascending, and max-min ≤ 1) instead of only relying on toEqual below. Example assertions you can add after this line:

expect(result).toHaveLength(2); expect(result.every(Number.isInteger)).toBe(true); expect(result.reduce((a, b) => a + b, 0)).toBe(6); expect(result).toEqual(result.slice().sort((a, b) => a - b)); expect(Math.max(...result) - Math.min(...result)).toBeLessThanOrEqual(1);

These ensure the test verifies the required properties from the task description, not only that the array matches a specific example.

mateacademy-ai-mentor · 2025-09-18T22:02:29Z

  if a value is divisible by a numberOfParts`, () => {
+  const result = splitInteger(6, 2);

+  expect(result).toEqual([3, 3]);


This expectation checks equality to [3, 3] which is a valid example check, but per the task description you must also assert the generic properties (length, integers, sum, sortedness, max-min <= 1). Either supplement this assertion with those property checks (recommended) or replace with property-based checks that will catch incorrect implementations that accidentally match the example but violate requirements.

mateacademy-ai-mentor · 2025-09-18T22:02:29Z


 test(`should return a part equals to a value
  when splitting into 1 part`, () => {
+  const result = splitInteger(8, 1);


For the splitInteger(8, 1) test add the property assertions after result is computed. Example assertions (adjust the expected value and length accordingly):

expect(result).toHaveLength(1); expect(result.every(Number.isInteger)).toBe(true); expect(result.reduce((a, b) => a + b, 0)).toBe(8); expect(result).toEqual(result.slice().sort((a, b) => a - b)); expect(Math.max(...result) - Math.min(...result)).toBeLessThanOrEqual(1);

This ensures the test aligns with the checklist items in the description.

mateacademy-ai-mentor · 2025-09-18T22:02:30Z

  when splitting into 1 part`, () => {
+  const result = splitInteger(8, 1);

+  expect(result).toEqual([8]);


The expect(result).toEqual([8]) assertion is fine as an example check, but it should be accompanied by the property assertions listed above to meet the explicit test requirements in the task description. Relying only on example equality does not fully validate the required properties.

mateacademy-ai-mentor · 2025-09-18T22:02:30Z

 });

 test('should sort parts ascending if they are not equal', () => {
+  const result = splitInteger(17, 4);


For the splitInteger(17, 4) test, compute property-based assertions after this line. Example:

expect(result).toHaveLength(4); expect(result.every(Number.isInteger)).toBe(true); expect(result.reduce((a, b) => a + b, 0)).toBe(17); expect(result).toEqual(result.slice().sort((a, b) => a - b)); expect(Math.max(...result) - Math.min(...result)).toBeLessThanOrEqual(1);

This complements the example equality and verifies the generic properties required by the task.

mateacademy-ai-mentor · 2025-09-18T22:02:30Z

 test('should sort parts ascending if they are not equal', () => {
+  const result = splitInteger(17, 4);

+  expect(result).toEqual([4, 4, 4, 5]);


The expect(result).toEqual([4, 4, 4, 5]) assertion verifies the example. Still, please add the property assertions so the test checks the general requirements (length, integer elements, sum, ascending order, and max-min difference).

mateacademy-ai-mentor · 2025-09-18T22:02:30Z

 });

 test('should add zeros if value < numberOfParts', () => {
+  const result = splitInteger(2, 4);


For the splitInteger(2, 4) test add property assertions after this line. Example assertions should use numberOfParts = 4 and value = 2 to confirm length, integer-ness, sum equals 2, ascending order, and max-min ≤ 1. This ensures the edge-like distribution (zeros included) satisfies the checklist requirements.

mateacademy-ai-mentor · 2025-09-18T22:02:30Z

 test('should add zeros if value < numberOfParts', () => {
+  const result = splitInteger(2, 4);

+  expect(result).toEqual([0, 0, 1, 1]);


The expectation toEqual([0, 0, 1, 1]) is a useful example for the value < numberOfParts scenario, but it should be supplemented with the property checks described earlier (length, all integers, sum, sorted, max-min ≤ 1). Also, add the missing example from the description splitInteger(32, 6) === [5,5,5,5,6,6] as a separate test to fully cover the listed examples in the task description.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Review Summary

Great work on adding the new test case and creating the assertProperties helper function! This is a big step forward.

However, this requires changes because a core requirement is not yet met. The task requires that the general properties of the result (like sum, length, and sort order) are checked for every test case. Currently, assertProperties is only called from the new (32, 6) test.

Please add a call to your assertProperties helper function inside each of the other four tests in src/splitInteger.test.js to ensure all scenarios are fully validated.

As a minor improvement, please also update .eslintrc.js to use the string 'off' instead of the number 0 for better readability. You're very close to completing this!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-10-22T15:21:47Z

-    'no-proto': 0,
-    strict: 0,
-    'max-len': 0,
+    'no-proto': 0


As mentioned in the previous review, it's more readable and conventional to use the string 'off' instead of the number 0 to disable an ESLint rule. While it works correctly, using the string makes the configuration easier to understand.

mateacademy-ai-mentor · 2025-10-22T15:21:48Z

  if a value is divisible by a numberOfParts`, () => {
+  const result = splitInteger(6, 2);

+  expect(result).toEqual([3, 3]);


It's good that you're checking for the exact output. However, per the requirements, you also need to verify the general properties of the result for each test case. Please add a call to your assertProperties helper function here, like you did for the (32, 6) test.

mateacademy-ai-mentor · 2025-10-22T15:21:48Z

  when splitting into 1 part`, () => {
+  const result = splitInteger(8, 1);

+  expect(result).toEqual([8]);


This test is also missing the call to assertProperties. Remember to check the general properties for all test cases.

mateacademy-ai-mentor · 2025-10-22T15:21:48Z

 test('should sort parts ascending if they are not equal', () => {
+  const result = splitInteger(17, 4);

+  expect(result).toEqual([4, 4, 4, 5]);


Please add the assertProperties call here as well to ensure this result satisfies all the required properties.

mateacademy-ai-mentor · 2025-10-22T15:21:48Z

 test('should add zeros if value < numberOfParts', () => {
+  const result = splitInteger(2, 4);
+
+  expect(result).toEqual([0, 0, 1, 1]);


This test also needs the property assertions. Please add a call to assertProperties to complete it.

mateacademy-ai-mentor

Review Summary

Excellent work on this revision! You've done a fantastic job implementing the assertProperties helper across all your tests, which makes them much more robust and fully compliant with the requirements. I am approving your solution as an educational override so you can move forward, but there is one small detail to keep in mind for the future. In .eslintrc.js, please use the string 'off' instead of the number 0 to disable rules for better readability and consistency with ESLint standards. Overall, this is a very strong submission. Great job!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2025-10-22T18:17:53Z

-    'no-proto': 0,
-    strict: 0,
-    'max-len': 0,
+    'no-proto': 0


This was mentioned in the previous review. For better readability and consistency with ESLint standards, please use the string 'off' instead of the number 0 to disable this rule.

js_split_integer

bbceb5c

mateacademy-ai-mentor suggested changes Sep 18, 2025

View reviewed changes

shelyaa requested a review from mateacademy-ai-mentor October 22, 2025 14:11

mateacademy-ai-mentor reviewed Oct 22, 2025

View reviewed changes

shelyaa requested a review from mateacademy-ai-mentor October 22, 2025 14:31

task

3c3bea7

mateacademy-ai-mentor suggested changes Oct 22, 2025

View reviewed changes

task

b49ea22

shelyaa requested a review from mateacademy-ai-mentor October 22, 2025 18:16

mateacademy-ai-mentor approved these changes Oct 22, 2025

View reviewed changes

Conversation

shelyaa commented Sep 18, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants