Skip to content
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

Add optional argument to split #489

Merged
merged 2 commits into from
Dec 28, 2021
Merged

Add optional argument to split #489

merged 2 commits into from
Dec 28, 2021

Conversation

Jason2605
Copy link
Member

Well detailed description of the change :

This PR adds a new optional argument to str.split() which determines the amount of times a string will actually be broken up by the delimiter. This allows us to keep the delimiter within the string after a certain amount of occurrences have already been found.

E.g

"Dictu is awesome!".split(" ", 0); // ['Dictu is awesome!']
"Dictu is awesome!".split(" ", 1); // ['Dictu', 'is awesome!']
"Dictu is awesome!".split(" ", 2); // ['Dictu', 'is', 'awesome!']

Type of change:

  • Bug fix

  • New feature

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Housekeeping

  • Tests have been updated to reflect the changes done within this PR (if applicable).

  • Documentation has been updated to reflect the changes done within this PR (if applicable).

Note

This may come in useful with regards to #473

@Jason2605 Jason2605 added the enhancement Implementation of a new feature label Dec 26, 2021
@Jason2605 Jason2605 self-assigned this Dec 26, 2021
@Jason2605 Jason2605 mentioned this pull request Dec 27, 2021
4 tasks
@briandowns
Copy link
Contributor

I'm curious about your thoughts on this:

Keep basic functionality and expectations the same.

"Dictu is awesome!".split(" "); // ['Dictu', 'is', 'awesome!']

Add another method, like Go, to indicate the number of times to split.

"Dictu is awesome!".splitN(" ", 1); // ['Dictu', 'is awesome!']
"Dictu is awesome!".splitN(" ", 2); // ['Dictu', 'is', 'awesome!']

or

Have value indicating all instances of the seperator.

"Dictu is awesome!".split(" ", -1); // ['Dictu', 'is', 'awesome!']

@Jason2605
Copy link
Member Author

Jason2605 commented Dec 27, 2021

Hey! Thanks for the comments!

Add another method, like Go, to indicate the number of times to split.

I think another method will probably end up with a lot of code duplication or macro abuse which are never fun to deal with (neither are strings in C at the best of times ;( )

.split(delim, occur) is also how it is in Python 😄

Have value indicating all instances of the separator.

This is something I don't mind too much about. Checking in Python a negative number seems to make it ignore the count and just split by all occurrences of the delimiter (like how you propose with -1). The current implementation treats negative numbers as not wanting to split (so essentially the same as passing 0). Probably a good idea as it means you can have a negative number as a default rather than having to have two flows in user code with:

// Flow 1
const count = cond ? 3 : -1;
".........".split(",", count);

// Flow 2
if (cond) {
    ".........".split(",");
} else {
    ".........".split(",", 3);
}

@Jason2605
Copy link
Member Author

@briandowns I've updated the PR to make negative numbers split all occurrences now, thanks for the thought!

@briandowns
Copy link
Contributor

LGTM!

@Jason2605 Jason2605 merged commit 9315dfe into develop Dec 28, 2021
@Jason2605 Jason2605 deleted the feature/str_split branch December 28, 2021 01:51
@Jason2605 Jason2605 mentioned this pull request Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Implementation of a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants