Skip to content
This repository was archived by the owner on Jun 13, 2018. It is now read-only.

refactoring some Guava ListenableFUture to Java 8 CompletableFuture #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yulin2
Copy link

@yulin2 yulin2 commented Jan 4, 2015

Hi, I'm doing research on new concurrent constructs in Java 8. CompletableFuture in Java 8 has the same functionality as Guava ListenableFuture. But CompletableFuture is much nicer because it comes together with Lambda expression in Java 8, and it is monadic, which makes the code more readable and cleaner. Also, it provides more ways to compose different tasks. Therefore, using CompletableFuture instead of ListenableFuture is better for future extension and maintenance of the code.

I tried to refactoring some ListenableFuture instances in trinityshell to CompletableFuture, so you can see the difference. You don't have to merge this pull request. I'm just wondering your opinion on this kind of refactoring (or migrating the code from Java 7 to Java 8). Do you think the refactoring is useful? Do you have any plan to use Java 8? Thanks.

public Optional<P> call() throws Exception {
return AbstractCachedProtocol.this.protocolCache.get(xWindow);
final CompletableFuture<Optional<P>> protocolFuture = CompletableFuture.supplyAsync(() -> {
return AbstractCachedProtocol.this.protocolCache.get(xWindow);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not guarantee that the task will be executed by the display executor and might give concurrency problems. Normally, in trinityshell, a POJO should only accessible by a single thread, as such, executors are single threaded and use queues to guarantee in order execution.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you say the task won't be executed by the display executor? The second argument of CompletableFuture.supplyAsync is the displayExecutor, so the provided task will be executed on the given displayExecutor.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you're correct

@Zubnix
Copy link
Owner

Zubnix commented Jan 6, 2015

To answer your questions:
-Do you think the refactoring is useful?
Certainly, standardizing and minimizing dependencies on other frameworks is always a good thing. However the master branch is no longer developed, instead development is focused on 0.1.0-SNAPSHOT branch.

Do you have any plan to use Java 8?
The project on the 0.1.0-SNAPSHOT branch already uses java8 (it's also a complete rewrite), the project on the master branch started when java8 was still in beta, the main reason why guava's optional and listenable future were used instead of java8 counterparts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants