esbtools / event-handler

Notification in, document out.
GNU General Public License v3.0
3 stars 6 forks source link

Allow getting response by index from requester #27

Closed alechenninger closed 8 years ago

kahowell commented 8 years ago

What is this for? Code looks fine, but would like a little more context.

alechenninger commented 8 years ago

Sure. I found this necessary if you have a situation where one of the requests sent to requester is conditional: you may or may not be requesting it. In that case, you pass a Collection to the requester (the previous PR). You can't necessarily refer to the request by instance due to scoping rules and the requirement that variables must by final (explicitly or implicitly) to be used in a lambda expression or anonymous class.

For example:

List<DataFindRequest> requests = new ArrayList(2);
DataFindRequest findFoo = findFooById(1);

requests.add(findFoo);

if (fooHasAFriend()) {
  DataFindRequest findFriendOfFoo = findFriendOfFooByFooId(1);
  requests.add(findFriendOfFoo);
}

requester.request(requests).transformSync(responses -> {
  LightblueDataResponse fooResponse = responses.forRequest(findFoo);
  Optional<LightblueDataResponse> = fooHasAFriend()
      ? Optional.of(responses.forRequest(1))
      : Optional.empty();

      // Do stuff with responses...
});

There are other ways to do it I suppose but I find this more or less one of the better ones for various reasons. Is there a better approach jumping out at you?

alechenninger commented 8 years ago

I found an alternative that I like better, although I still think by index may be useful in the future, because I am abusing the ternary operator to ensure the request object is [implicitly] final. You could probably get around that in other sane ways though if you had to.

List<DataFindRequest> requests = new ArrayList(2);
DataFindRequest findFoo = findFooById(1);

requests.add(findFoo);

Optional<DataFindRequest> maybeFindFriendOfFoo = fooHasAFriend()
    ? Optional.of(findFriendOfFooByFooId(1))
    : Optional.empty();

maybeFindFriendOfFoo.ifPresent(requests::add);

requester.request(requests).transformSync(responses -> {
  LightblueDataResponse fooResponse = responses.forRequest(findFoo);
  Optional<LightblueDataResponse> = maybeFindAFriendOfFoo.map(responses::forRequest);

  // Do stuff with responses...
});

If you'd rather not have the option of by index, I wouldn't protest too much since above should be possible in most situations I can think of.

alechenninger commented 8 years ago

The thing I like much better about revised example is that the conditional is kept in one place instead of needing to be checked twice in two places.

alechenninger commented 8 years ago

I'm gonna close this for now, I think we can reopen if it turns out we need this.