7

please be advised, this is a long post. Sorry for that but I want to make my point clear:

I was wondering how to separate Swing GUI from Presentation and Business Logic for quite a long time. At work I had to implement a 3 MD Excel Export for some data with a small Swing Dialog to configure the export. We do not use a framework like Spring for this so I had to implement it myself.

I wanted to completely separate GUI from Business Logic, which are in precise following tasks:

  • Tell BL to start its job from GUI
  • Report Progress from BL to GUI
  • Report Logging from BL to GUI
  • Delegate BL Result to GUI

of course the GUI shouldnt have notice of the BL implementation and vice versa. I created several interfaces for all those tasks above, e. g. a ProgressListener, LogMessageListener, JobDoneListener, etc., to be fired by the Business Logic. For instance, if the Business Logic wants to tell about logging, it calls

fireLogListeners("Job has been started");

classes that implement the public interface LogListener + are attached to the BL, now will be notified about the "Job has been started" log message. All these listeners are at this time implemented by the GUI itself, which in general looks like this:

public class ExportDialog extends JDialog implements ProgressListener, LogListener, JobFinishedListener,  ErrorListener {

    @Override
    public void jobFinished(Object result){
        // Create Save File dialog and save exported Data to file.
    }

    @Override
    public void reportProgress(int steps){
        progressBar.setValue(progressBar.getValue()+steps);
    }

    @Override
    public void errorOccured(Exception ex, String additionalMessage){
        ExceptionDialog dialog = new ExceptionDialog(additionalMessage, ex);
        dialog.open();
    }

    // etc.
}

The "GUI and BL creating class" simply attaches the GUI (as all these listeners' interface) to the BL, which looks something like this:

exportJob.addProgressListener(uiDialog);
exportJob.addLogListener(uiDialog);
exportJob.addJobFinishedListener(uiDialog);
exportJob.start();

I am now quite unsure about that because it looks weird because of all those newly created listener Interfaces. What do you think about? How do you separate your Swing GUI components from BL?

Edit: For better demonstrating purpose I created a Demo workspace in eclipse file-upload.net/download-9065013/exampleWorkspace.zip.html I pasted it to pastebin also, but better import those classes in eclipse, pretty a lot of code http://pastebin.com/LR51UmMp

aterai
  • 9,658
  • 4
  • 35
  • 44
Stefano L
  • 1,486
  • 2
  • 15
  • 36
  • 2
    The direction you're heading in seems reasonable to me. Your idea to provide event notification via a observer pattern is reasonable, I also don't disagree with the separation of the listeners in this way, as it provides you with the flexibility to change who gets notified of what without overlapping responsibilities. Coding to interface is a good approach as well. About the only thing you could and is a factory pattern to load the BL logic via some kind of dynamic loading process, but otherwise, I think you're moving in the right direction – MadProgrammer Jun 15 '14 at 10:12
  • 1
    I would look at the [MVP](http://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93presenter) pattern. This is the most common way of layering a GUI application. Your approach is half way there, but I think creating the additional layer will add much more flexibility. i.e. you need to split the View from the Presenter. – Boris the Spider Jun 15 '14 at 10:16
  • 1
    I like your design! Your export job is nicely decouppled, you have no swing components in there. – myborobudur Jun 15 '14 at 10:23
  • 2
    Your design is fine, but you have to be careful about threading issues: the business logic shouldn't run in the Swing event dispatch thread. But the Swing components should always be used from the event dispatch thread. So your listeners should using SwingUtilities.invokeLater() to update the UI when they receive an event from the business logic. – JB Nizet Jun 15 '14 at 11:53
  • a full compilable example would be good. just include one method then we can analyze the mechanics. – Oliver Watkins Jun 15 '14 at 12:16
  • I create an example now. Will post it here when its finished – Stefano L Jun 15 '14 at 12:36
  • @OliverWatkins I created a compiling example in eclipse. Here you go, eclipse workspace: http://www.file-upload.net/download-9065013/exampleWorkspace.zip.html does it work? – Stefano L Jun 15 '14 at 13:27
  • the downloadable file is executable. sorry i cant – Oliver Watkins Jun 15 '14 at 13:33
  • sorry i cant trust that its not going to do me some damage – Oliver Watkins Jun 15 '14 at 13:33
  • @OliverWatkins ok no problem, i pasted it to pastebin, better import those classes in eclipse, pretty a lot of code http://pastebin.com/LR51UmMp – Stefano L Jun 15 '14 at 13:37
  • got it. ill post an answer with my thoughts soon – Oliver Watkins Jun 15 '14 at 13:43
  • @JBNizet thanks for your reply. actually i run the business logic in a asynchronous thread. Classes that implement the BL Listeners must implement a method called "shouldRunInUIThread". For my UI Dialog Class, that MUST handle all calls of the listeners in UI Thread, this method simply returns true for all listeners. see my code example for better understanding. I am quite unsure about this approach since the BL itself calls SwingUtilies.invokeLater, not UI. On the other hand, if the UI always calls SwingUtilies, every call would use it, but maybe its already called from UI Thread...? – Stefano L Jun 15 '14 at 13:53
  • THat's quite a strong coupling between the UI and the BL. The BL should not care about Swing: that's not its responsibility. The listeners should crae about it, because they are the ones which deal with the UI. – JB Nizet Jun 15 '14 at 15:42
  • how do I know then in the UI, if the method had been called from UI thread or another external thread, boolean isAsyncCall = true in methods signature? or even call SwingUtilies.invokeLater() every time? – Stefano L Jun 15 '14 at 17:30
  • but in general i agree to your point, because if the ExportJob should be used by some no-UI class, the SwingUtilities if-query is obsolete or could even crash the application if not used correctly – Stefano L Jun 15 '14 at 17:39

2 Answers2

2

A few things.

I would no have the uiDialog code in the ExportFunction class. The whole perform method should just be code in the main class. The ExportFunctions responsibility is to 'export' not to 'show gui'.

public static void main(String[] args) {
    ExportFunction exporter = new ExportFunction();

    final ExportUIDialog uiDialog = new ExportUIDialog();
    uiDialog.addActionPerformedListener(exporter);

    uiDialog.pack();
    uiDialog.setVisible(true);
}

(Swing.invokeLater() is not needed)

You seem to be overengineering a fair bit. I don't know why you would expect to have many threads to be running at the same time. When you press the button, you would only expect one thread to run right? Then there would be no need to have an array of actionPerformedListener.

instead of this :

button.addActionListener(new ActionListener() {

    @Override
    public void actionPerformed(ActionEvent arg0) {
        if (startConditionsFulfilled()) {
            fireActionListener(ActionPerformedListener.STARTJOB);
        }
    }

});

why not just :

final ExportJob exportJob = new ExportJob();
exportJob.addJobFinishedListener(this);
exportJob.addLogListener(this);

button.addActionListener(new ActionListener() {

    @Override
    public void actionPerformed(ActionEvent e) {
        exportJob.start();
    }
});

That way you can get rid of ExportFunction which doesn't really serve any purpose.

You seem to have a lot of arrays of listeners. Unless you really really need them I wouldn't bother with them and keep it as simple as possible.

Instead of :

Thread.sleep(1000);
fireLogListener("Excel Sheet 2 created");
Thread.sleep(1000);

Just have :

Thread.sleep(1000);
log("Excelt Sheet 1 created");
Thread.sleep(1000);

where log is :

private void log(final String message) {
    ((DefaultListModel<String>) list.getModel()).addElement(message);
}

This way you are keeping it simpler and cleaner.

GUI should not know about BL, but BL somehow has to tell the GUI what to do. You can abstract ad infinitum with lots of interfaces, but in 99.99% of applications this is not necessary, especially yours which seems fairly simple.

So while the code you have written is pretty good, i would try and simplify and reduce the interfaces. It doesn't warrant that much engineering.

Oliver Watkins
  • 12,575
  • 33
  • 119
  • 225
  • Hi, now I think the application is well-coupled. I cannot replace the Business Logic implementation "ExportJob", e. g. with a subclass because its the hardcoded ExportJob. Thats what MadProgrammer meant by "FactoryPattern", to be more flexible in changing the business logic implementation. Also, you binded the BL directly to the UIDialog class if i understand the log() method approach directly in the ExportJob class correctly. – Stefano L Jun 15 '14 at 17:34
  • you can reduce the binding on ExportJob if you make the Controller handle the event listening. That way you would only have a reference from controller to UI. I think the point i am trying to make is, that unless you are trying to develop a framework (like Eclipse OSGi) then it is not necessary to abstract things with interfaces like you are. Just my opinion. For me its just a thread that updates a few GUI components that could be written in half the code that you have. – Oliver Watkins Jun 15 '14 at 20:04
2

Basically, your architecture seems ok to me. I suppose you wonder if it is because of the numerous listeners you set up.

A solution for this might be either:

a) to have a generic Event class, with subclasses for specific events. You could use a visitor to implement the actual listeners.

b) to use an Event Bus (see guava, for instance). With an event bus architecture, your model will publish events to the event bus, and your UI objects will listen for events from the event bus, and filter them.

Some systems can even use annotations for declaring listener methods.

khaemuaset
  • 217
  • 1
  • 9