ReactiveX / RxCpp

Reactive Extensions for C++
Apache License 2.0
3.07k stars 396 forks source link

Feature: group_by_until #445

Closed diorcety closed 6 years ago

diorcety commented 6 years ago

Rxcpp misses group_by_until operator like the rxpy one https://github.com/ReactiveX/RxPY/blob/master/rx/linq/observable/groupbyuntil.py This allow to clear the mapping, which is important for long running applications

kirkshoop commented 6 years ago

This would be a great addition. Right now I do this by using timeout() or take_until on each group. this has the same result, but with more flexibility. timeout() makes an MRU based lifetime while take_until() is a max window based lifetime.

diorcety commented 6 years ago

I have a prototype here: https://github.com/diorcety/RxCpp/commit/1f5ace5e65e25a749f87e9f4ff473e881349c454

kirkshoop commented 6 years ago

looks good! I added a few comments to the commit.

diorcety commented 6 years ago

This is inspired from the rxpy implementation. I do that because, without any expire, the mapping always growing in my case (so take_until not resolve this issue)

kirkshoop commented 6 years ago

If you add group | take_until(timer(1s)) to each group as it is created, then when the timer fires, I expect that group to be removed. If you add group | timeout(1s) to each group as it is created, then the timeout will stop the group when a second goes by without an item in that group. I you add both to each group as it is created, then you can build even more complex semantics for group reclamation.

diorcety commented 6 years ago

How it can be removed from state->groups without erase

kirkshoop commented 6 years ago

yep, you are correct, group_by does not remove from the map when the group is unobserved. I did not remember that. the implementation you had is needed. You can ignore the take_until comment.

diorcety commented 6 years ago

I made some updates. For example here: https://github.com/diorcety/RxCpp/commit/ae2c3274009c3f1883f7fadf5b60c1503ea2e84a#diff-9430ecceb5eeb688548de212b7fd57beR228

diorcety commented 6 years ago

The naming is ugly, feel free to ask for modifications or to rework it directly.

kirkshoop commented 6 years ago

I have only a couple of comments. otherwise it looks ready for a pr to me..