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 takeWhileInclusive operator #370

Closed
shinayser opened this issue Nov 22, 2019 · 10 comments
Closed

Add takeWhileInclusive operator #370

shinayser opened this issue Nov 22, 2019 · 10 comments

Comments

@shinayser
Copy link

As discussed here:

closeAfterEmitting does not exist, but it might make sense for it to exist since other Rx implementations have something along this line: takeWhile(myPredicate, inclusive: true). Since takeWhile comes from the Stream Interface, we'd have to create a new method called takeWhileInclusive or something.

Originally posted by @shinayser in #369 (comment)

@frankpepermans
Copy link
Member

frankpepermans commented Dec 15, 2019

So basically this is a takeWhile, but it also adds the next event before closing?

So imagine:

Stream.fromIterable([0, 1, 2, 3, 4, 5, 6, 7])
      .takeWhileInclusive((i) => i < 5)
      .listen(print); // 0, 1, 2, 3, 4, [5] done

Should include also have effect the other way around? Take:

Stream.fromIterable([0, 1, 2, 3, 4, 5, 6, 7])
      .takeWhileInclusive((i) => i > 1)
      .listen(print); // [1], 2, 3, 4, 5, 6, 7 done

What about this one?

Stream.fromIterable([0, 1, 2, 3, 4, 5, 6, 7])
      .takeWhileInclusive((i) => i > 1 && i < 5)
      .listen(print); // [1], 2, 3, 4, [5] done

@shinayser
Copy link
Author

shinayser commented Dec 17, 2019

Tricky question...

There are 2 possible ways of implementing it.

Case A.

Closing immediatly after receiving the element that "falses" the condition.
So, in the second example it should print: 1, close
In the bottom example it should print 1, done.

Case B.

Ignore elements until it finds an element that matches the condition. Then, after finding it, it starts respecting to the case A.
So in the second example, it should print: 2, 3, 4, 5, 6, 7, done
In the bottom it should print: 2, 3, 4, 5, done

Perhaps a boolean parameter to the function could inform what kind of behavior the user wants. Something like that:

myStream.takeWhileInclusive( (i) => i > 1, skipUntil: true);

You could add our examples to the method documentation. I think it will be great that way :)

@frankpepermans
Copy link
Member

frankpepermans commented Dec 18, 2019

For case B, something like ignoreWhile might be better?

Stream.fromIterable([0, 1, 2, 3, 4, 5, 6, 7])
      .ignoreWhile((i) => i < 2)
      .listen(print); // 3, 4, 5, 6, 7 done
Stream.fromIterable([0, 1, 2, 3, 4, 5, 6, 7])
      .ignoreWhileInclusive((i) => i < 2)
      .listen(print); // 2, 3, 4, 5, 6, 7 done

...which complements takeWhile and takeWhileInclusive

Basically if it is an inverted takeWhile, then it should not close when the condition yields true a second time:

Stream.fromIterable([0, 1, 2, 3, 0, 1, 2, 3])
      .ignoreWhile((i) => i < 2)
      .listen(print); // 3, 0, 1, 2, 3 done

@shinayser
Copy link
Author

shinayser commented Dec 18, 2019

But if ignoreWhile doesn't close it will be basically a where, doesn't it?

@frankpepermans
Copy link
Member

No, just as takeWhile stops as soon as the condition fires, ignoreWhile waits until the condition fires.

So in both cases, they're 1 time toggles.

A where simply filters.

Take Stream.fromIterable([1,2,3,1 2 3])
takeWhile i < 3 yields 1,2,done
ignoreWhile i < 3 yield 3,1,2,3,done
where i < 3 finally yields 1,2,1,2,done

@shinayser
Copy link
Author

Nice. I think this is a good approach.
So, implementing Case A with a new ignoreWhile method?

@frankpepermans
Copy link
Member

Yeah I'll do a PR tomorrow for both, if ignoreWhile is relevant enough, it can join :)

@shinayser
Copy link
Author

Yeah I'll do a PR tomorrow for both, if ignoreWhile is relevant enough, it can join :)

Sure it is! I can't think in a working example for it right now but I am sure it can be usefull for some situations.

@hoc081098
Copy link
Collaborator

Consider name skipWhile instead of ignoreWhile
https://rxjs-dev.firebaseapp.com/api/operators/skipWhile

@shinayser
Copy link
Author

I agree with @hoc081098!
Keeping the same naming will be important for new users.

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

3 participants