Skip to content

Added array.reduce - #604

Merged
Perryvw merged 7 commits into
masterfrom
feature/reduce
May 27, 2019
Merged

Added array.reduce#604
Perryvw merged 7 commits into
masterfrom
feature/reduce

Conversation

@Perryvw

@Perryvw Perryvw commented May 26, 2019

Copy link
Copy Markdown
Member

No description provided.

Comment thread src/lualib/ArrayReduce.ts Outdated
Perryvw and others added 2 commits May 26, 2019 14:41
Co-Authored-By: ark120202 <ark120202@gmail.com>
@Perryvw
Perryvw requested a review from tomblind May 26, 2019 15:19
Comment thread test/unit/lualib/array.spec.ts Outdated
`return JSONStringify([${inp.toString()}].slice(${start}, ${end}))`,
);

expect(result).toBe(JSON.stringify(inp.slice(start, end)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It might be good to test calling slice with no arguments (since that's a common way to do an array copy).

{ inp: [0, 1, 2, 3], start: 4, deleteCount: 1, newElements: [8, 9] },
{ inp: [0, 1, 2, 3], start: 4, deleteCount: 0, newElements: [8, 9] },
{ inp: [0, 1, 2, 3, 4, 5], start: 5, deleteCount: 9, newElements: [10, 11] },
{ inp: [0, 1, 2, 3, 4, 5], start: 3, deleteCount: 2, newElements: [3, 4, 5] },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should test negative start indicies and undefined deleteCount.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

undefined deleteCount?

{ inp: [0, 1, 2, 3], start: 4 },
{ inp: [0, 1, 2, 3, 4, 5], start: 3 },
{ inp: [0, 1, 2, 3, 4, 5], start: 2, deleteCount: 2 },
{ inp: [0, 1, 2, 3, 4, 5, 6, 7, 8], start: 5, deleteCount: 9, newElements: [10, 11] },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Like above, it would probably be good to test negative start indicies here.

@Perryvw
Perryvw merged commit 440ef27 into master May 27, 2019
@Perryvw
Perryvw deleted the feature/reduce branch May 27, 2019 19:27
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.

3 participants