-
Notifications
You must be signed in to change notification settings - Fork 1.1k
my solution #1013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
my solution #1013
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,4 +20,6 @@ function splitInteger(value, numberOfParts) { | |
| return parts; | ||
| } | ||
|
|
||
| // console.log(splitInteger(12, 0)); | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the extra blank/commentable marker line. Keeping stray blank/commentable lines is harmless but removing it keeps the file tidy. |
||
| module.exports = splitInteger; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,18 +4,61 @@ | |
|
|
||
| test(`should split a number into equal parts | ||
| if a value is divisible by a numberOfParts`, () => { | ||
|
|
||
| expect(splitInteger(12, 3)).toEqual([4, 4, 4]); | ||
| }); | ||
|
|
||
| test(`should return a part equals to a value | ||
| when splitting into 1 part`, () => { | ||
|
|
||
| expect(splitInteger(8, 1)).toEqual([8]); | ||
| }); | ||
|
|
||
| test('should sort parts ascending if they are not equal', () => { | ||
| expect(splitInteger(8, 3)).toEqual([2, 3, 3]); | ||
| expect(splitInteger(17, 4)).toEqual([4, 4, 4, 5]); | ||
| }); | ||
|
|
||
| test('should always return an array of length numberOfParts', () => { | ||
| expect(splitInteger(32, 6)).toHaveLength(6); | ||
| }); | ||
|
|
||
| test('should return an array with integer in every case', () => { | ||
| expect(splitInteger(5, 3).every(Number.isInteger)).toBe(true); | ||
| expect(splitInteger(5, 10).every(Number.isInteger)).toBe(true); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: This assertion uses |
||
| }); | ||
|
|
||
| test('should sum of parts always equal to value', () => { | ||
| const value = 17; | ||
|
|
||
| expect(splitInteger(value, 3).reduce((a, b) => a + b, 0)).toBe(value); | ||
| expect(splitInteger(value, 10).reduce((a, b) => a + b, 0)).toBe(value); | ||
| expect(splitInteger(value, 1).reduce((a, b) => a + b, 0)).toBe(value); | ||
| }); | ||
|
|
||
| test('should add zeros if value < numberOfParts', () => { | ||
| test('should deference between max and min part to be <= 1', () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: Typo in test title — "deference" should be "difference". Fixing this improves clarity of the test description but does not affect behavior. |
||
| if (splitInteger().length > 0) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: This conditional uses a call with no arguments and causes the assertions inside to be skipped. Calling |
||
| expect(Math.max(...splitInteger(17, 3)) - Math.min(...splitInteger(17, 3))) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion to make assertions more robust/clear: avoid calling |
||
| .toBeLessThanOrEqual(1); | ||
|
|
||
| expect(Math.max(...splitInteger(17, 10)) - Math.min(...splitInteger(17, 10))) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar suggestion for the other max/min assertions: store |
||
| .toBeLessThanOrEqual(1); | ||
|
|
||
| expect(Math.max(...splitInteger(17, 4)) - Math.min(...splitInteger(17, 4))) | ||
| .toBeLessThanOrEqual(1); | ||
| } | ||
| }); | ||
|
|
||
| test('should return an array is sorted in non-decreasing order', () => { | ||
| expect(splitInteger(32, 6)).toEqual(splitInteger(32, 6).slice() | ||
| .sort((a, b) => a - b)); | ||
|
|
||
| expect(splitInteger(17, 4)).toEqual(splitInteger(17, 4).slice() | ||
| .sort((a, b) => a - b)); | ||
|
|
||
| expect(splitInteger(8, 1)).toEqual(splitInteger(8, 1).slice() | ||
| .sort((a, b) => a - b)); | ||
| }); | ||
|
|
||
| test('should return an array with amount of larger parts to equal value % numberOfParts if not evenly divisible ', () => { | ||
| expect(splitInteger(17, 4).filter((part) => part === 5)).toHaveLength(1); | ||
| expect(splitInteger(32, 6).filter((part) => part === 6)).toHaveLength(2); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the commented-out debugging line
// console.log(splitInteger(12, 0));. It is unnecessary in production/test code and may confuse reviewers. The function already exports correctly, so the log can be deleted. (Line currently commented out.)