{"action":"created","issue":{"url":"https://api.github.com/repos/ReactiveX/RxJava/issues/5038","repository_url":"https://api.github.com/repos/ReactiveX/RxJava","labels_url":"https://api.github.com/repos/ReactiveX/RxJava/issues/5038/labels{/name}","comments_url":"https://api.github.com/repos/ReactiveX/RxJava/issues/5038/comments","events_url":"https://api.github.com/repos/ReactiveX/RxJava/issues/5038/events","html_url":"https://github.com/ReactiveX/RxJava/issues/5038","id":204514444,"number":5038,"title":"tryOnNext design","user":{"login":"davidmoten","id":318187,"avatar_url":"https://avatars.githubusercontent.com/u/318187?v=3","gravatar_id":"","url":"https://api.github.com/users/davidmoten","html_url":"https://github.com/davidmoten","followers_url":"https://api.github.com/users/davidmoten/followers","following_url":"https://api.github.com/users/davidmoten/following{/other_user}","gists_url":"https://api.github.com/users/davidmoten/gists{/gist_id}","starred_url":"https://api.github.com/users/davidmoten/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidmoten/subscriptions","organizations_url":"https://api.github.com/users/davidmoten/orgs","repos_url":"https://api.github.com/users/davidmoten/repos","events_url":"https://api.github.com/users/davidmoten/events{/privacy}","received_events_url":"https://api.github.com/users/davidmoten/received_events","type":"User","site_admin":false},"labels":[{"id":480313802,"url":"https://api.github.com/repos/ReactiveX/RxJava/labels/2.x","name":"2.x","color":"af40af","default":false},{"id":24234455,"url":"https://api.github.com/repos/ReactiveX/RxJava/labels/Question","name":"Question","color":"cc317c","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2017-02-01T07:27:52Z","updated_at":"2017-02-01T09:22:59Z","closed_at":null,"body":"I have written an operator in *rxjava2-extras* called [`collectWhile`](https://github.com/davidmoten/rxjava2-extras#collectwhile). I'd appreciate a review for any glaring problems especially as this is my first foray into supporting an aspect of operator fusion, namely *request-one* micro-fusion. Source for the operator is [here](https://github.com/davidmoten/rxjava2-extras/blob/master/src/main/java/com/github/davidmoten/rx2/internal/flowable/FlowableCollectWhile.java).\r\n\r\nBy the way I'm happy with how the RxJava 2 api hangs together for writing operators. Super job.\r\n\r\nI'm a bit curious about `tryOnNext`. When something happens for that method to call `onError` and terminate it's unclear what I should return. I assume it's actually more performant to encourage another loop of a `while (e!=r)` loop upstream by returning false from `tryOnNext` so that we encounter the `cancelled` check straight away instead of possibly (if `e==r`) hitting the atomics `requested` and `this` outside the inner loop and then doing the `cancelled` check.\r\n\r\nPerhaps the `tryOnNext` method could return an enum with values like `EMITTED`,`NOT_EMITTED`,`CANCELLED` and the `onNext` method would look like \r\n\r\n```java\r\nif (tryOnNext(t) == NOT_EMITTED) {\r\n    parent.request(1);\r\n}\r\n```\r\n\r\nThe upstream drain loop might avoid a `cancelled` check by doing:\r\n\r\n```java\r\nResult result = child.tryOnNext(t);\r\nif (result == Result.EMITTED) {\r\n    e++;\r\n} else if (result == Result.CANCELLED) {\r\n  return;\r\n}\r\n```\r\nI'm not wedded to those names of course, they are just so you get the idea.\r\n\r\nDo you see any advantages to changes like these?"},"comment":{"url":"https://api.github.com/repos/ReactiveX/RxJava/issues/comments/276609852","html_url":"https://github.com/ReactiveX/RxJava/issues/5038#issuecomment-276609852","issue_url":"https://api.github.com/repos/ReactiveX/RxJava/issues/5038","id":276609852,"user":{"login":"davidmoten","id":318187,"avatar_url":"https://avatars.githubusercontent.com/u/318187?v=3","gravatar_id":"","url":"https://api.github.com/users/davidmoten","html_url":"https://github.com/davidmoten","followers_url":"https://api.github.com/users/davidmoten/followers","following_url":"https://api.github.com/users/davidmoten/following{/other_user}","gists_url":"https://api.github.com/users/davidmoten/gists{/gist_id}","starred_url":"https://api.github.com/users/davidmoten/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidmoten/subscriptions","organizations_url":"https://api.github.com/users/davidmoten/orgs","repos_url":"https://api.github.com/users/davidmoten/repos","events_url":"https://api.github.com/users/davidmoten/events{/privacy}","received_events_url":"https://api.github.com/users/davidmoten/received_events","type":"User","site_admin":false},"created_at":"2017-02-01T09:22:59Z","updated_at":"2017-02-01T09:22:59Z","body":">L162: you conditionally offer to the queue after indicating you are done. This may lose the very last collection because a typical drain will find done && queue.isEmpty(). You should set done just before drain.\r\nThanks, yep\r\n>L188: checking done after a poll() may leave unconsumed collections if they appear between the two, always read done first into a local variable, check ˙poll/isEmpty`.\r\nTa\r\n>L209: the case for if (e == r) is missing here; it covers the case when the upstream ends on a request boundary and can emit an onComplete() without the need to request more.\r\nYep, that would be nice to have, I was lazy.\r\n>L224: I don't think you need queue-drain here at all. You can have it flat and whenever you don't emit a collection in tryOnNext, just return false indicating the upstream should produce another value. See FlowableBufferPredicate.\r\nYep I though the queue drain was overkill, just being lazy.\r\n\r\nThanks for the review (I need to be a bit more careful! I'll check other operators for similar probs).\r\n\r\n"}}
{"actor":{"display_login":"davidmoten"}}