Skip to content

Refactor Task execution to support asynchronous callbacks #3274

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Refactor Task execution to support asynchronous callbacks #3274

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d157d6
refactor: Refactor Task execution to support asynchronous callbacks
HY-love-sleep May 3, 2025
b42dc28
fix: CallbackTest
HY-love-sleep May 3, 2025
12a7f1d
chore: code style
HY-love-sleep May 3, 2025
5f8069b
Merge remote-tracking branch 'upstream/master'
HY-love-sleep May 17, 2025
a86c211
feat: display synchronous and asynchronous callbacks
HY-love-sleep May 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion callback/src/main/java/com/iluwatar/callback/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ public final class App {
private App() {}

/** Program entry point. */
public static void main(final String[] args) {
public static void main(final String[] args) throws InterruptedException {
var task = new SimpleTask();
task.executeWith(() -> LOGGER.info("I'm done now."));
Thread.sleep(3000);
}
}
13 changes: 9 additions & 4 deletions callback/src/main/java/com/iluwatar/callback/Task.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,20 @@
*/
package com.iluwatar.callback;

import java.util.Optional;
import java.util.concurrent.CompletableFuture;

/** Template-method class for callback hook execution. */
public abstract class Task {

/** Execute with callback. */
/** Execute the task and asynchronously call the callback method upon completion. */
final void executeWith(Callback callback) {
execute();
Optional.ofNullable(callback).ifPresent(Callback::call);
CompletableFuture.runAsync(
() -> {
execute();
if (callback != null) {
callback.call();
}
});
Copy link
Owner

Choose a reason for hiding this comment

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

In general, looks good! However, I'd like to keep the synchronous callbacks (previous functionality) as well.

Could we demonstrate both sychronous and asynchronous callback in the example? The guidance would be something like

  • If your operation is short and straightforward, keep it synchronous.
  • If the operation can block or is resource-intensive, prefer asynchronous callbacks to maintain application responsiveness.

}

public abstract void execute();
Expand Down
24 changes: 19 additions & 5 deletions callback/src/test/java/com/iluwatar/callback/CallbackTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;

/**
Expand All @@ -39,19 +41,31 @@ class CallbackTest {
private Integer callingCount = 0;

@Test
void test() {
Callback callback = () -> callingCount++;
void test() throws InterruptedException {
CountDownLatch latch = new CountDownLatch(1);

var task = new SimpleTask();
CountDownLatch finalLatch = latch;
Callback callback =
() -> {
callingCount++;
finalLatch.countDown();
};

assertEquals(Integer.valueOf(0), callingCount, "Initial calling count of 0");
var task = new SimpleTask();

task.executeWith(callback);

latch.await(5, TimeUnit.SECONDS);
Copy link

Choose a reason for hiding this comment

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

The test uses await(5, TimeUnit.SECONDS) which might lead to flaky tests if the task takes longer than 5 seconds to complete. Consider using a more robust mechanism for waiting, such as a timeout with a more appropriate duration or a different synchronization primitive.


assertEquals(Integer.valueOf(1), callingCount, "Callback called once");

callingCount = 0;
latch = new CountDownLatch(1);

task.executeWith(callback);

assertEquals(Integer.valueOf(2), callingCount, "Callback called twice");
latch.await(5, TimeUnit.SECONDS);

assertEquals(Integer.valueOf(1), callingCount, "Callback called once again");
}
Copy link

Choose a reason for hiding this comment

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

The test uses await(5, TimeUnit.SECONDS), which might lead to flaky tests if the task takes longer than 5 seconds. Consider using a more robust waiting mechanism, such as a timeout with a more appropriate duration or a different synchronization primitive.

}
Loading