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

Introduce EndpointResult #707

Merged
merged 1 commit into from Dec 25, 2016
Merged

Introduce EndpointResult #707

merged 1 commit into from Dec 25, 2016

Conversation

vkostyukov
Copy link
Member

@vkostyukov vkostyukov commented Dec 23, 2016

Currently, Endpoints are backed by Option[(Input, Rerrunable[Output[A]])]s. There is nothing wrong with that type and it provides a great insight on what happens in the endpoint, but there is a way to make it 1) simpler and 2) faster.

Let's flattenOption[Tuple2[_, _]] into a new type EndpointResult (ADT with two cases: matched vs. skipped) and save some allocations on option creations.

In addition to that this PR cleans up a couple of places that I think were poorly structured:

  1. All the testing methods on endpoint's result are now explicit (no need for implicit syntax class over the Option).
  2. All blocking methods are renamed (deprecated) to awaitX variants to visually signal the reader that the call is blocking. They also take optional Duration, which comes in handy in tests.

UPDATE: I decided to give up on a mutable EndpointResult for now since it doesn't buy much of performance improvement but tradeoffs immutability. The most recent benchmark run is here (https://gist.github.com/vkostyukov/e54092f843280dc3f8930c1d359db342). TL;DR EndpointResult[_] performs better or same as Option[Tuple2[_, _]].

@codecov-io
Copy link

codecov-io commented Dec 23, 2016

Current coverage is 76.85% (diff: 74.04%)

Merging #707 into master will decrease coverage by 0.36%

@@             master       #707   diff @@
==========================================
  Files            33         34     +1   
  Lines           641        661    +20   
  Methods         618        635    +17   
  Messages          0          0          
  Branches         23         26     +3   
==========================================
+ Hits            495        508    +13   
- Misses          146        153     +7   
  Partials          0          0          

Powered by Codecov. Last update 136dbe4...f68f0ae

@vkostyukov vkostyukov mentioned this pull request Dec 24, 2016
@vkostyukov vkostyukov changed the title Introduce EndpointResult [WIP] Introduce EndpointResult Dec 25, 2016
@vkostyukov vkostyukov merged commit d16d769 into master Dec 25, 2016
2 of 4 checks passed
@vkostyukov vkostyukov deleted the vk/endpoint-result branch Dec 25, 2016
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

Successfully merging this pull request may close these issues.

None yet

2 participants