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.

Sign up for GitHub

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

Jump to bottom

Add methods to Async to allow using method references in ambiguous situations #386

Closed
sleberknight opened this issue Oct 29, 2020 · 0 comments · Fixed by #387
Closed

Add methods to Async to allow using method references in ambiguous situations #386

sleberknight opened this issue Oct 29, 2020 · 0 comments · Fixed by #387
Assignees
@sleberknight
Labels
enhancement A request for change or improvement to an existing feature
Milestone
0.14.0

Comments

@sleberknight
Copy link
Member

In another library we have the following code:

return withMaxTimeout(doAsync(() -> invoker.get()), supplierTimeout, supplierTimeoutUnit).get();

SonarCloud is complaining that we should use a method reference for () -> invoker.get() but if you try, you receive the following compilation error:

[ERROR] ...elided.../Projects/dropwizard-client-poller/src/main/java/org/kiwiproject/dropwizard/poller/ClientPoller.java:[326,35] reference to doAsync is ambiguous
[ERROR]   both method <T>doAsync(java.util.function.Supplier<T>) in org.kiwiproject.concurrent.Async and method doAsync(java.lang.Runnable) in org.kiwiproject.concurrent.Async match

We should add methods to Async to allow calling the Runnable or Supplier methods unambiguously.

Suggestions:

  • runAsync
  • supplyAsync

Hopefully it is obvious which one takes what argument. 😉
@sleberknight sleberknight added the enhancement A request for change or improvement to an existing feature label Oct 29, 2020
@sleberknight sleberknight self-assigned this Nov 2, 2020
@sleberknight sleberknight added this to the 0.14.0 milestone Nov 2, 2020
sleberknight added a commit that referenced this issue Nov 2, 2020
@sleberknight
Add runAsync and supplyAsync methods to Async
cf90879 
These methods are aliases for the corresponding doAsync methods.
We found cases (in the dropwizard-client-poller library) where the
compiler was unable to determine whether a method reference was a
Runnable or Supplier and we had to use a lambda instead. Adding these
provides a way in ambiguous situations to disambiguate. And frankly
they are probably better names anyway, and they are named the same
as similar methods in CompletableFuture (which might be a good or a
bad thing depending on context).

Changes:
* Add runAsync(Runnable) and runAsync(Runnable, Executor)
* Add supplyAsync(Runnable) and supplyAsync(Runnable, Executor)
* Restructure AsyncTest to use @nested style grouped by method; oddly
  I had to increase the timeout in one test, probably due to the
  additional overhead of all the nested classes. I increased it from
  150 to 250 millis just to have an additional margin of safety. This
  was only necessary when running tests with coverage.

Fixes #386
@sleberknight sleberknight mentioned this issue Nov 2, 2020
Merged
chrisrohr pushed a commit that referenced this issue Nov 2, 2020
@sleberknight
Add runAsync and supplyAsync methods to Async (#387)
e769e05 
These methods are aliases for the corresponding doAsync methods.
We found cases (in the dropwizard-client-poller library) where the
compiler was unable to determine whether a method reference was a
Runnable or Supplier and we had to use a lambda instead. Adding these
provides a way in ambiguous situations to disambiguate. And frankly
they are probably better names anyway, and they are named the same
as similar methods in CompletableFuture (which might be a good or a
bad thing depending on context).

Changes:
* Add runAsync(Runnable) and runAsync(Runnable, Executor)
* Add supplyAsync(Runnable) and supplyAsync(Runnable, Executor)
* Restructure AsyncTest to use @nested style grouped by method; oddly
  I had to increase the timeout in one test, probably due to the
  additional overhead of all the nested classes. I increased it from
  150 to 250 millis just to have an additional margin of safety. This
  was only necessary when running tests with coverage.

Fixes #386
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sleberknight sleberknight

Labels
enhancement A request for change or improvement to an existing feature
Projects
None yet
Milestone
0.14.0
Development

Successfully merging a pull request may close this issue.

Add runAsync and supplyAsync methods to Async kiwiproject/kiwi
1 participant
@sleberknight