2010-06-21 6 views
16

diesen Code vor:Wie kann dieser Code Swingworker gemacht wird prüfbar

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 ein JTextField ist und ‚Schöpfer‘ ist eine injizierte Schnittstelle zum Schreiben der Datei (so dass ein Teil ist im Test). Der Ort, an dem die Datei geschrieben wird, ist absichtlich hart codiert, da dies als Beispiel dienen soll. Und java.util.logging wird einfach verwendet, um eine externe Abhängigkeit zu vermeiden.

Wie würden Sie das aufteilen, um es unitestfähig zu machen (einschließlich des Verzichts auf SwingWorker bei Bedarf, aber dann Ersetzen seiner Funktionalität, zumindest wie hier verwendet).

So wie ich es sehe, ist der doInBackground grundsätzlich in Ordnung. Die grundlegenden Mechanismen erstellen einen Schreiber und schließen ihn, was fast zu einfach zu testen ist und die wirkliche Arbeit wird getestet. Die done-Methode ist jedoch problematisch, einschließlich ihrer Kopplung mit der actionPerformed-Methode der übergeordneten Klasse und der Koordination der Aktivierung und Deaktivierung der Schaltfläche.

Allerdings ist das Auseinanderziehen nicht offensichtlich. Durch das Einfügen einer Art von SwingWorkerFactory ist es viel schwieriger, die GUI-Felder zu erfassen (es ist schwer zu erkennen, wie dies eine Designverbesserung wäre). Das JOpitonPane und der Desktop haben alle die "Güte" von Singletons, und die Ausnahmebehandlung macht es unmöglich, das Get einfach zu verpacken.

Also, was wäre eine gute Lösung, um diesen Code unter Test zu bringen?

+0

Umformatierter Code; Bitte zurück, wenn nicht korrekt. – trashgod

+1

Keine vollständige Antwort: Aber wenn Sie Qualitätscode mögen, gehen Sie nicht in die Nähe von 'SwingWorker'. Im Allgemeinen Faktoren aus. Wo Sie eine API haben, die Statik/Singletons verwendet, führen Sie eine Schnittstelle mit einer Implementierung ein, die die "echte" statische API verwendet, und eine andere für das Spotten (möglicherweise eine andere für das Auditing). –

+0

@Tom, wenn Sie Zeit haben, den Entwurf eines alternativen Entwurfs zu SwingWorker zu schreiben (oder wenn Sie von einer alternativen besseren Implementierung wissen) würde es sehr geschätzt werden. – Yishai

Antwort

10

IMHO, das ist kompliziert für eine anonyme Klasse. Mein Ansatz wäre es, die anonyme Klasse, so etwas Refactoring:

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); 
     } 
    } 
} 

, dass die Datei zu schreiben Funktionalität ermöglicht

Dann eines GUI unabhängig getestet werden, wird die actionPerformed etwas wie folgt aus:

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(); 
} 

Zusätzlich kann die Instanz FileWriterWorker.Response einer Variablen zugewiesen und unabhängig von FileWriterWorker getestet werden.

+0

Es gibt einige nette Ideen/Idiome in diesem Code-Snippet. Danke, ich werde es ein bisschen kauen. – Yishai

+0

Ich liebe das einfach. – gdbj

-1

Einfache Lösung: ein einfacher Timer ist am besten; Sie lanchieren Ihren Timer, Sie starten Ihre actionPerformed, und bei der Zeitüberschreitung muss der Bouton aktiviert werden und so weiter.

Hier ist ein sehr littel exemple mit einem java.util.Timer ist:

package goodies; 

import java.util.Timer; 
import java.util.TimerTask; 
import javax.swing.JButton; 

public class SWTest 
{ 
    static class WithButton 
    { 
    JButton button = new JButton(); 

    class Worker extends javax.swing.SwingWorker<Void, Void> 
    { 
     @Override 
     protected Void doInBackground() throws Exception 
     { 
     synchronized (this) 
     { 
      wait(4000); 
     } 
     return null; 
     } 

     @Override 
     protected void done() 
     { 
     button.setEnabled(true); 
     } 
    } 

    void startWorker() 
    { 
     Worker work = new Worker(); 
     work.execute(); 
    } 
    } 

    public static void main(String[] args) 
    { 
     final WithButton with; 
     TimerTask verif; 

     with = new WithButton(); 
     with.button.setEnabled(false); 
     Timer tim = new Timer(); 
     verif = new java.util.TimerTask() 
     { 
     @Override 
     public void run() 
     { 
      if (!with.button.isEnabled()) 
      System.out.println("BAD"); 
      else 
      System.out.println("GOOD"); 
      System.exit(0); 
     }}; 
     tim.schedule(verif, 5000); 
     with.startWorker(); 
    } 
} 

Vermeintliche Expert Lösung: ein Swing-Worker ist ein RunnableFuture, im Inneren ein FutureTask in einem abrufbaren Imbeded, so können Sie Verwenden Sie Ihren eigenen Executor, um es zu starten (RunableFuture). Dafür benötigen Sie einen SwingWorker mit einer Namensklasse, nicht einem anonymen. Mit Ihrem eigenen Executor und einer Namensklasse können Sie alles testen, was Sie wollen, sagt der vermeintliche Experte.

+1

Ich verstehe nicht, warum Sie einen Timer verwenden würden, um die Schaltfläche wieder zu aktivieren, wenn der Prozess in dieser Zeit möglicherweise nicht beendet wurde, oder die Schaltfläche nicht unnötig deaktiviert zu lassen. – Yishai

+0

Sorry, es ist mein schlechtes Englisch. Es ist nicht "muss aktiviert sein", sondern "ist möglich", nehme ich an. Ich bearbeite meine Antwort mit einem Java-Code. Ich hoffe es ist das Beste. – Istao

8

Die aktuelle Implementierung verbindet Threading-Probleme, UI und Datei-Schreiben - und wie Sie festgestellt haben, macht es die Kopplung schwierig, die einzelnen Komponenten isoliert zu testen.

Dies ist eine ziemlich lange Antwort, aber es läuft darauf hinaus, diese drei Bedenken aus der aktuellen Implementierung in separate Klassen mit einer definierten Schnittstelle herauszuziehen.

Factor aus Anwendungslogik

mit starten, konzentrieren sich auf die Anwendungslogik Kern und in eine separate Klasse/Schnittstelle bewegen. Eine Schnittstelle ermöglicht ein einfacheres Mocking und die Verwendung anderer Swing-Threading-Frameworks. Die Trennung bedeutet, dass Sie Ihre Anwendungslogik völlig unabhängig von den anderen Problemen testen können.

interface FileWriter 
{ 
    void writeFile(File outputFile, String location, Creator creator) 
     throws IOException; 
    // you could also create your own exception type to avoid the checked exception. 

    // a request object allows all the params to be encapsulated in one object. 
    // this makes chaining services easier. See later. 
    void writeFile(FileWriteRequest writeRequest); 
} 

class FileWriteRequest 
{ 
    File outputFile; 
    String location; 
    Creator creator; 
    // constructor, getters etc.. 
} 


class DefualtFileWriter implements FileWriter 
{ 
    // this is basically the code from doInBackground() 
    public File writeFile(File outputFile, String location, Creator creator) 
     throws IOException 
    { 
      Writer writer = null; 
      try { 
       writer = new FileWriter(outputFile); 
       creator.write(location, writer); 
      } finally { 
       if (writer != null) { 
        writer.close(); 
       } 
      } 
      return file; 
    } 
    public void writeFile(FileWriterRequest request) { 
     writeFile(request.outputFile, request.location, request.creator); 
    } 
} 

UI trennen

Mit der Anwendungslogik nun getrennt, wir ausklammern dann den Erfolg und die Fehlerbehandlung. Dies bedeutet, dass die Benutzeroberfläche getestet werden kann, ohne tatsächlich das Schreiben der Datei durchzuführen. Insbesondere kann die Fehlerbehandlung getestet werden, ohne dass tatsächlich diese Fehler provoziert werden müssen. Hier sind die Fehler ziemlich einfach, aber oft können einige Fehler sehr schwer zu provozieren sein. Durch die Trennung der Fehlerbehandlung besteht auch die Möglichkeit zur Wiederverwendung oder zum Ersetzen der Fehlerbehandlung. Z.B. mit einem JXErrorPane später.

interface FileWriterHandler { 
    void done(); 
    void handleFileWritten(File file); 
    void handleFileWriteError(Throwable t); 
} 

class FileWriterJOptionPaneOpenDesktopHandler implements FileWriterHandler 
{ 
    private JFrame owner; 
    private JComponent enableMe; 

    public void done() { enableMe.setEnabled(true); } 

    public void handleFileWritten(File file) { 
     try { 
     JOptionPane.showMessageDialog(owner, 
        "File has been retrieved and saved to:\n" 
        + file.getAbsolutePath()); 
     Desktop.getDesktop().open(file); 
     } 
     catch (IOException ex) { 
      handleDesktopOpenError(ex); 
     } 
    } 

    public void handleDesktopOpenError(IOException ex) { 
     logger.log(Level.INFO, "Unable to open file for viewing.", ex);   
    } 

    public void handleFileWriteError(Throwable t) { 
     if (t instanceof InterruptedException) { 
       logger.log(Level.INFO, "Thread interupted, process aborting.", ex); 
       // no point interrupting the EDT thread 
     } 
     else if (t instanceof ExecutionException) { 
      Throwable cause = ex.getCause() == null ? ex : ex.getCause(); 
      handleGeneralError(cause); 
     } 
     else 
     handleGeneralError(t); 
    } 

    public void handleGeneralError(Throwable cause) { 
     logger.log(Level.SEVERE, "An exception occurred that was " 
        + "not supposed to happen.", cause); 
     JOptionPane.showMessageDialog(owner, "Error: " 
        + cause.getClass().getSimpleName() + " " 
        + cause.getMessage(), "Error", JOptionPane.ERROR_MESSAGE); 
    } 
} 

Schließlich Threading trennen, können wir trennen auch die Threading Bedenken mit einem FileWriterService. Die Verwendung eines FileWriteRequest oben vereinfacht die Codierung.

interface FileWriterService 
{ 
    // rather than have separate parms for file writing, it is 
    void handleWriteRequest(FileWriteRequest request, FileWriter writer, FileWriterHandler handler); 
} 

class SwingWorkerFileWriterService 
    implements FileWriterService 
{ 
    void handleWriteRequest(FileWriteRequest request, FileWriter writer, FileWriterHandler handler) { 
     Worker worker = new Worker(request, fileWriter, fileWriterHandler); 
     worker.execute(); 
    } 

    static class Worker extends SwingWorker<File,Void> { 
     // set in constructor 
     private FileWriter fileWriter; 
     private FileWriterHandler fileWriterHandler; 
     private FileWriterRequest fileWriterRequest; 

     protected File doInBackground() { 
      return fileWriter.writeFile(fileWriterRequest); 
     } 
     protected void done() { 
      fileWriterHandler.done(); 
      try 
      { 
       File f = get(); 
       fileWriterHandler.handleFileWritten(f); 
      } 
      catch (Exception ex) 
      {     
       // you could also specifically unwrap the ExecutorException here, since that 
       // is specific to the service implementation using SwingWorker/Executors. 
       fileWriterHandler.handleFileError(ex); 
      } 
     } 
    } 

} 

Jeder Teil des Systems ist separat prüfbar - die Anwendungslogik, die Präsentation (Erfolg und Fehlerbehandlung) und die Threading-Implementierung ist auch eine separate Sorge.

Dies mag wie eine Menge von Schnittstellen scheinen, aber die Implementierung ist meist Cut-and-Paste aus Ihrem ursprünglichen Code. Die Schnittstellen bieten die Trennung, die erforderlich ist, um diese Klassen testbar zu machen.

Ich bin kein großer Fan von SwingWorker, also halten sie hinter einer Schnittstelle, um die Unordnung, die sie produzieren, aus dem Code zu halten. Sie können auch eine andere Implementierung zum Implementieren der separaten UI-/Hintergrundthreads verwenden. Um beispielsweise Spin zu verwenden, müssen Sie nur eine neue Implementierung von FileWriterService bereitstellen.

Verwandte Themen