Skip to content

Documentation example of split doesn't work#40

Closed
jake-lively wants to merge 4 commits intoReactiveX:1.xfrom
jake-lively:patch-1
Closed

Documentation example of split doesn't work#40
jake-lively wants to merge 4 commits intoReactiveX:1.xfrom
jake-lively:patch-1

Conversation

@jake-lively
Copy link
Copy Markdown

@jake-lively jake-lively commented May 15, 2016

I wasn't super sure I was following your test functions so I just tested the observables by transforming them to and from arrays.

Part of the problem is StringObservable.split() seems to always behave like String.split() limit 0 which trims excess empty strings at the end. There also doesn't seem to be a way to change the limit so at least the documentation needs to be updated to show this behavior.

Changing the limit in testSplit() doesn't help which is why I went with the array tests.
Limit -1 or even something like 6 should preserve the empty strings if you follow the normal String.split().
But that limit could be for something else. I didn't dive that deep in the test code.

I'm wasn't super sure I was following your test functions so I just tested the observables by transforming them to and from arrays.

Part of the problem is StringObservable.split() seems to always behave like String.split() limit 0 which trims excess empty strings at the end. There also doesn't seem to be a way to change the limit so at least the documentation needs to be updated to show this behavior.

Changing the limit in testSplit() doesn't help which is why I went with the array tests.
Limit -1 or even something like 6 should preserve the empty strings if you follow the normal String.split().
But that limit could be for something else. I didn't dive that deep in the test code.
It still failed because of different lengths. But now it should behave if they were actually equal.
It should pass all the current tests.
@akarnokd
Copy link
Copy Markdown
Member

/cc @davidmoten

@davidmoten
Copy link
Copy Markdown
Contributor

Thanks for this and excuse the delay. I'll run some examples with this myself but it certainly does appear that their is a conflict of intent in the operator that you have probably sorted out. I'll get back to you next week as I'm on holiday. Cheers.

@jake-lively
Copy link
Copy Markdown
Author

Did this ever get resolved one way or the other? I guess I should have made an issue for it.

@akarnokd
Copy link
Copy Markdown
Member

#47 has a fresh implementation.

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