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

until/takeUntil with predicate function #427

Closed
OliverJAsh opened this issue Apr 17, 2017 · 11 comments
Closed

until/takeUntil with predicate function #427

OliverJAsh opened this issue Apr 17, 2017 · 11 comments

Comments

@OliverJAsh
Copy link

The until/takeUntil function accepts an input stream, however I was wondering if we could add support for a predicate function? This is different from takeWhile because it is inclusive of the first item to return false.

For inspiration, have a look at the API for RxJava which has similar options (stream or predicate) for its takeUntil operator: http://reactivex.io/RxJava/javadoc/rx/Observable.html#takeUntil(rx.functions.Func1)

@briancavalier
Copy link
Member

Hi @OliverJAsh. I see, yes, there's a subtle difference around the end. Do you have a concrete use case for it? If you can share it, that might help us think about how broadly useful this kind of operation is, and whether it belongs in the core or another package.

Thanks!

@OliverJAsh
Copy link
Author

OliverJAsh commented Apr 18, 2017

Hi @briancavalier!

My use case is paginating an API response of ordered items until an item exceeds the range, for example: https://gist.github.com/OliverJAsh/2c327ae63941a237594eed34fe60a47b#file-foo-js-L97

I have an async generator that iterates the pages of Twitter’s home_timeline API. I want take the pages until the end of the user's timeline for today has been found (e.g. takeUntil(page => page.some(tweet => tweet.created_at < new Date()))).

@briancavalier
Copy link
Member

Thanks for the additional info @OliverJAsh. I see what you mean about inclusive vs. exclusive. At first glance, it seems possible to use takeWhile plus every, but that isn't inclusive, so you'd only get all the desired pages if the date aligned exactly with a page boundary.

We're trying to be more strict with types, and avoid runtime type checks, so I'd rather not overload the name takeUntil, even though that name is deprecated (but not yet removed). Haskell calls it dropWhileEnd.

Random thoughts about names: skipAfter, endAfter, takeUpto ... other ideas?

@OliverJAsh
Copy link
Author

I had a look at dropWhileEnd and I don't understand the example well enough to say whether or not it is equivalent to what I have in mind.

There is a similar discussion on the subject of naming in this thread on RxJava: ReactiveX/RxJava#1649

@briancavalier
Copy link
Member

I kinda like the name skipAfter. It reads fairly nicely in prose: "skip all the events after 'Fred'", and in code: events.skipAfter(x => x === 'Fred').

Trying to think of a name using "take" that describes this operation, without using the word "until" is tricky!

I think takeUntil, or anything using "until" with a predicate could be confusing, because it doesn't necessarily have a strong indication of whether it's inclusive or exclusive (that concern is also expressed in the RxJava thread). "When" has a similar problem, imho. I think the word "after" has a more clear connotation. "After" has a time connotation, which could lead someone to believe it's like until or since, but "while" has the same issue. Maybe then it's helpful if we stick with "until" and "since" as time vocabulary, and "while" and "after" as predicate vocabulary?

Other ideas?

cc @davidchase @TylorS

@davidchase
Copy link
Collaborator

skipAfter reads really nice and concise 👍

@Frikki
Copy link
Collaborator

Frikki commented Apr 20, 2017

I also favor skipAfter for the same reasons.

@briancavalier
Copy link
Member

Cool, thanks @davidchase and @Frikki. Let's go with skipAfter. I should have time to PR a first version in the next couple days.

@briancavalier
Copy link
Member

Hey @OliverJAsh, we just released 1.3.0 with skipAfter. Check it out, and let us know if it works for you.

@OliverJAsh
Copy link
Author

Thanks @briancavalier, I'll let you know!

@briancavalier
Copy link
Member

Hi @OliverJAsh. Closing, but please reopen if there's something more we need to discuss or do.

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

No branches or pull requests

4 participants