Hi, Rémi,

thanks for your feedback. See more comments below.

On 8/24/2010 2:36 PM, Rémi Forax wrote:
Le 24/08/2010 10:57, [email protected] a écrit :
Changeset: 7e26538596be
Author: art
Date: 2010-08-24 12:54 +0400
URL: http://hg.openjdk.java.net/jdk7/awt/jdk/rev/7e26538596be

6949936: Provide API for running nested events loops, similar to what
modal dialogs do
Reviewed-by: ant, anthony

! src/share/classes/java/awt/Dialog.java
! src/share/classes/java/awt/EventDispatchThread.java
! src/share/classes/java/awt/EventQueue.java
+ src/share/classes/java/awt/SecondaryLoop.java
+ src/share/classes/java/awt/WaitDispatchSupport.java
+ test/java/awt/EventQueue/SecondaryLoopTest/SecondaryLoopTest.java


Hi all, nice to see that this kind of API find its way to jdk7.

The example provided in SecondaryLoop don't compile.
Here is a slightly modified version that compile.

The code in JavaDoc is not a complete test case, but rather a kind of stub. To make it compile, you need to declare 'loop' final or make it a class field, put the code with ActionListener into a method, etc.

JButton jButton = new JButton("Button");
jButton.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
EventQueue queue = Toolkit.getDefaultToolkit()
.getSystemEventQueue();
final SecondaryLoop loop = queue.createSecondaryLoop();

Runnable runnable = new Runnable() {
@Override
public void run() {
// Perform calculations
doSomethingUseful();

// Exit the loop
loop.exit();
}
};

// Spawn a new thread to do the work
Thread worker = new Thread(runnable);
worker.start();

// Enter the loop to block the current event
// handler, but leave UI responsive
if (!loop.enter()) {
// Report an error
}
}
});

About the API design, SecondaryLoop.enter is not reentrant so it's
an error to call it recursively. So enter() should throw a runtime
exception
instead of returning false. I propose to use IllegalStateException.

Well, these two approaches are roughly the same, but throwing an exception usually makes the code a bit less clear.

SecondaryLoop is in my opinion a bad name, a for or a while is a loop,
in my opinion, SecondaryEventLoop is a better name.

Also, you don't provide a way to transfer any value from the computation
done in the thread back to the event queue thread like by example
the SwingWorker does.

I propose to change SecondaryEventLoop to:

public interface SecondaryEventLoop<V> {
public V enter();
public boolean exit(V value);
}

exit() can send a value that will be returned when enter() will end.

I also wonder if the return value of exit is useful.
exit can be called before enter() due to a race between enter() and exit(),
why a developer should care about that.

SecondaryLoop is different from SwingWorker: it shouldn't be used to perform a task and obtain a value. Its only purpose is to make sure UI is not frozen (EDT keeps running even if the current event handler is blocked), but it doesn't spawn any new threads and doesn't delegate any work to other threads.

cheers,
Rémi

Thanks,

Artem

Reply via email to