Feature/custom test by mlabucki · Pull Request #1058 · mate-academy/js_split_integer

mlabucki · 2026-01-16T19:29:53Z

helpers in customs

mateacademy-ai-mentor

Review Summary

Good effort on building out the test suite for splitInteger. However, a few changes are needed to meet all the requirements.

Your test suite is missing two required examples from the task description: splitInteger(17, 4) and splitInteger(32, 6). Please ensure all specified examples are tested directly. Additionally, your custom helper toBeSortedAscending in src/customs.js has a logical error that makes the sorting test unreliable.

To simplify your tests and make them more robust, I recommend replacing the complex custom helpers with Jest's built-in toEqual matcher. A single assertion like expect(splitInteger(6, 2)).toEqual([3, 3]) is clearer and verifies the array's content, length, and order all at once.

✨ 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 · 2026-01-16T19:31:43Z

src/customs.js

+function toBeSortedAscending(arr) {
+  if (hasRemainder(arr) === false) {
+    return arr.every((val, i) => i === 0 || arr[i - 1] <= val);
+  }
+}


This toBeSortedAscending function has a couple of significant issues:

The hasRemainder function is called with arr as an argument. However, hasRemainder is defined to accept two numbers, not an array. This is an incorrect use of the function.

If the if condition evaluates to false, this function doesn't return any value, which means it implicitly returns undefined. A function designed for checking a condition should always return true or false.

Consider simplifying this function to only perform the sorting check, as that's its main purpose.

mateacademy-ai-mentor · 2026-01-16T19:31:43Z

src/splitInteger.test.js

+  const result = splitInteger(34, 2);
+  const resultOfCustoms = hasSpilitEqual(result, 34, 2);

+  expect(resultOfCustoms).toEqual(true);


It's great that you're testing this case. However, using a custom helper function makes the test indirect. A simpler and more direct assertion would be expect(splitInteger(6, 2)).toEqual([3, 3]), which directly tests against the example from the requirements.

changed into direct assertion

mateacademy-ai-mentor · 2026-01-16T19:31:43Z

src/splitInteger.test.js

+  const result = splitInteger(34, 1);
+  const resultOfCustoms = hasEqualValueToNumber(result, 34);

+  expect(resultOfCustoms).toEqual([34]);


While this assertion works, using the specific example from the requirements (splitInteger(8, 1)) would be better. Also, you can simplify this by removing the custom helper: expect(splitInteger(8, 1)).toEqual([8]).

The helpers from customs will be discussed on the meetong with mentor. Need more info about proper pattern of helpers and siply assertions.

mateacademy-ai-mentor · 2026-01-16T19:31:43Z

src/splitInteger.test.js

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

+  expect(toBeSortedAscending(result)).toBe(true);


This test correctly checks if the array is sorted, which is one of the requirements. However, it's missing validation for the actual content of the array. The task requires testing specific examples like splitInteger(17, 4). A single assertion expect(splitInteger(17, 4)).toEqual([4, 4, 4, 5]) would verify sorting, correct values, length, and the max-min difference all at once.

mateacademy-ai-mentor · 2026-01-16T19:31:43Z

src/splitInteger.test.js

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

+  expect(zerosCounts(2, 4)).toBe(result);


This assertion is not robust enough because it only checks the count of zeros in the result, not the full array content. The actual result for splitInteger(2, 4) is [0, 0, 1, 1]. A better test would be expect(splitInteger(2, 4)).toEqual([0, 0, 1, 1]) to ensure the entire output is correct.

mateacademy-ai-mentor

Review Summary

I am approving this submission as an educational override to help you move forward, but there are a couple of important items to address to fully complete the task.

You've done an excellent job simplifying your tests by using Jest's built-in toEqual matcher, which makes them much cleaner! To finish up, please remember to add the required test case for splitInteger(32, 6). Also, consider updating the last test that uses the hasEqualValueToNumber helper to use toEqual for consistency. This change will allow you to remove the now-unnecessary src/customs.js file.

Keep up the great work!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-01-16T19:51:55Z

src/customs.js

+    return arr.every((val, i) => i === 0 || arr[i - 1] <= val);
+  }
+
+  return true;


This part of the logic is flawed because it unconditionally returns true when a number is evenly divisible, without actually checking if the array is sorted. It's great that you're no longer using this function, and removing it entirely would be the best solution.

mateacademy-ai-mentor · 2026-01-16T19:51:55Z

src/customs.js

+}
+
+function hasEqualValueToNumber(result, number) {
+  return result[0] === number ? result : false;


This helper makes the test that uses it more complex than necessary. A direct assertion like expect(result).toEqual([8]) in the test file is much clearer. By updating that test, you can remove this helper and eventually this entire file.

mateacademy-ai-mentor · 2026-01-16T19:51:55Z

src/splitInteger.test.js

+  const result = splitInteger(8, 1);
+  const resultOfCustoms = hasEqualValueToNumber(result, 8);

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


Great job simplifying the other tests to use toEqual! For consistency, you can simplify this test as well. A single line expect(splitInteger(8, 1)).toEqual([8]); would be clearer and achieve the same goal without needing the custom helper.

mlabucki · 2026-01-16T19:53:42Z

Question related to customs/helpers

mlabucki added 2 commits January 16, 2026 20:06

Solution

46c1598

Re-run CI

65ad90e

mateacademy-ai-mentor suggested changes Jan 16, 2026

View reviewed changes

Solution with simply assertions

e0a0a17

mlabucki requested a review from mateacademy-ai-mentor January 16, 2026 19:50

mateacademy-ai-mentor approved these changes Jan 16, 2026

View reviewed changes

DmytroOleksiiuk approved these changes Jan 17, 2026

View reviewed changes

Conversation

mlabucki commented Jan 16, 2026

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!

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!

mlabucki commented Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants