Consider this code:
public void actionPerformed(ActionEvent e) {
setEnabled(false);
new SwingWorker<File, Void>() {
private String location = url.getText();
@Override
protected File doInBackground() throws Exception {
File file = new File("out.txt");
Writer writer = null;
try {
writer = new FileWriter(file);
creator.write(location, writer);
} finally {
if (writer != null) {
writer.close();
}
}
return file;
}
@Override
protected void done() {
setEnabled(true);
try {
File file = get();
JOptionPane.showMessageDialog(FileInputFrame.this,
"File has been retrieved and saved to:\n"
+ file.getAbsolutePath());
Desktop.getDesktop().open(file);
} catch (InterruptedException ex) {
logger.log(Level.INFO, "Thread interupted, process aborting.", ex);
Thread.currentThread().interrupt();
} catch (ExecutionException ex) {
Throwable cause = ex.getCause() == null ? ex : ex.getCause();
logger.log(Level.SEVERE, "An exception occurred that was "
+ "not supposed to happen.", cause);
JOptionPane.showMessageDialog(FileInputFrame.this, "Error: "
+ cause.getClass().getSimpleName() + " "
+ cause.getMessage(), "Error", JOptionPane.ERROR_MESSAGE);
} catch (IOException ex) {
logger.log(Level.INFO, "Unable to open file for viewing.", ex);
}
}
}.execute();
url
is a JTextField and 'creator' is an injected interface for writing the file (so that part is under test). The location where the file is written is hard coded on purpose because this is intended as an example. And java.util.logging is used simply to avoid an external dependency.
How would you chunk this up to make it unit-testable (including abandoning SwingWorker if needed, but then replacing its functionality, at least as used here).
The way I look at it, the doInBackground is basically alright. The fundamental mechanics are creating a writer and closing it, which is almost too simple to test and the real work is under test. However, the done method is quote problematic, including its coupling with the actionPerformed method the parent class and coordinating the enabling and disabling of the button.
However, pulling that apart is not obvious. Injecting some kind of SwingWorkerFactory makes capturing the GUI fields a lot harder to maintain (it is hard to see how it would be a design improvement). The JOpitonPane and the Desktop have all the "goodness" of Singletons, and exception handling makes it impossible to wrap the get easily.
So what would be a good solution to bring this code under test?
SwingWorker is designed for situations where you need to have a long running task run in a background thread and provide updates to the UI either when done, or while processing. Subclasses of SwingWorker must implement the doInBackground() method to perform the background computation.
If the SwingWorker object has not finished executing the doInBackground() method, the call to this method blocks until the result is ready. It is not suggested to call this method on the event dispatch thread, as it will block all events until it returns. cancels the task if it is still running.
Since Swing is not thread-safe by design, it's designer did provide couple of utility methods in SwingUtilities class to update any Swing component from a thread other thread Event Dispatcher Thread.
IMHO, that's complicated for an anonymous class. My approach would be to refactor the anonymous class to something like this:
public class FileWriterWorker extends SwingWorker<File, Void> {
private final String location;
private final Response target;
private final Object creator;
public FileWriterWorker(Object creator, String location, Response target) {
this.creator = creator;
this.location = location;
this.target = target;
}
@Override
protected File doInBackground() throws Exception {
File file = new File("out.txt");
Writer writer = null;
try {
writer = new FileWriter(file);
creator.write(location, writer);
}
finally {
if (writer != null) {
writer.close();
}
}
return file;
}
@Override
protected void done() {
try {
File file = get();
target.success(file);
}
catch (InterruptedException ex) {
target.failure(new BackgroundException(ex));
}
catch (ExecutionException ex) {
target.failure(new BackgroundException(ex));
}
}
public interface Response {
void success(File f);
void failure(BackgroundException ex);
}
public class BackgroundException extends Exception {
public BackgroundException(Throwable cause) {
super(cause);
}
}
}
That allows the file writing functionality to be tested independent of a GUI
Then, the actionPerformed
becomes something like this:
public void actionPerformed(ActionEvent e) {
setEnabled(false);
Object creator;
new FileWriterWorker(creator, url.getText(), new FileWriterWorker.Response() {
@Override
public void failure(FileWriterWorker.BackgroundException ex) {
setEnabled(true);
Throwable bgCause = ex.getCause();
if (bgCause instanceof InterruptedException) {
logger.log(Level.INFO, "Thread interupted, process aborting.", bgCause);
Thread.currentThread().interrupt();
}
else if (cause instanceof ExecutionException) {
Throwable cause = bgCause.getCause() == null ? bgCause : bgCause.getCause();
logger.log(Level.SEVERE, "An exception occurred that was "
+ "not supposed to happen.", cause);
JOptionPane.showMessageDialog(FileInputFrame.this, "Error: "
+ cause.getClass().getSimpleName() + " "
+ cause.getMessage(), "Error", JOptionPane.ERROR_MESSAGE);
}
}
@Override
public void success(File f) {
setEnabled(true);
JOptionPane.showMessageDialog(FileInputFrame.this,
"File has been retrieved and saved to:\n"
+ file.getAbsolutePath());
try {
Desktop.getDesktop().open(file);
}
catch (IOException iOException) {
logger.log(Level.INFO, "Unable to open file for viewing.", ex);
}
}
}).execute();
}
Additionally, the instance of FileWriterWorker.Response
can be assigned to a variable and tested independent of FileWriterWorker
.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With