3

I am developing a swing app, where I have a Factory class which provide Component keeping Singleton in mind. Like:

public final class ComponentFactory {
    private static LibraryFrame libraryFrame;
    private static LibraryTableScrollPane libraryTableScrollPane;

    public static synchronized LibraryFrame getLibraryFrame() {
        if (libraryFrame == null) {
            libraryFrame = new LibraryFrame();
        }
        return libraryFrame;
    }

    public static synchronized LibraryTableScrollPane getLibraryTableScrollPane() {     
        if(libraryTableScrollPane == null) {
            libraryTableScrollPane = new LibraryTableScrollPane(getLibraryTable());
        }       
        return libraryTableScrollPane;
    }
}

I am using this component as:

add(ComponentFactory.getLibraryTableScrollPane())

Also I make a ListenerFactory class which provides various Listeners of Swing/AWT.

Is this pattern has any flaws? Can I use a same component or listener with two concurrently visible parent component?

Thanks in advance.

mKorbel
  • 109,525
  • 20
  • 134
  • 319
Tapas Bose
  • 28,796
  • 74
  • 215
  • 331

2 Answers2

8

It has a major flaw: it promotes a lack of encapsulation by making every component globally accessible. This can lead very quickly to spaghetti code where every object uses any other object, instead of having a short list of dependencies providing encapsulated methods.

Another problem is with the implementation: the synchronization is unnecessary, since Swing components are not thread-safe, and may only be used from the event dispatch thread. You should thus only have the EDT calling your methods, which makes synchronization unnecessary.

Finally, a component may only have one parent component. If the same compoentnmust be displayed in two different frames, for example, you'll need two instances of this component.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
0

Apart from the coupling problems that come with the singleton pattern (= many classes in your program have a dependency on your factory -> If your factory changes, many parts of your system are affected.), your singleton factory should work in a multi-threaded context.

But be careful not to optimize it. There is a technique called double-checked locking that was used to optimize your solution to get a higher degree of concurrency, but it has very subtle problems. If you are interested, see this declaration (and note people who signed it): http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

To get rid of the coupling to your factory, I would rather create the shared structures (tables, listeners, frames) in some top-level class(es) that also create(s) the objects that need references to these structures and pass the structures into their constructors. But that is just an advice, I do not know the overall structure of the program.

joergl
  • 2,850
  • 4
  • 36
  • 42
  • -1: Swing components are only usable in a single thread: the EDT. And the OP's code is already thread-safe. Using double-checked locking would only make it more fragile. – JB Nizet Feb 19 '12 at 10:17
  • If you actually read the answer, you see that I told him to avoid double-checked locking. Hint: "But be careful not to optimize it", "..., but it has very subtle problems". And there is nothing that prevents you from modifying Swing components from a thread different from the EDT. See: http://java.sun.com/developer/technicalArticles/Threads/swing/ "If you modify Swing component data from any thread other than the event dispatching thread, you must take precautions to ensure data integrity". I don't see why the answer deservers a downvote, but whatever... – joergl Feb 19 '12 at 18:41
  • This article is frm 2001, and is completely obsolete. Read http://docs.oracle.com/javase/7/docs/api/javax/swing/package-summary.html#threading instead: *All Swing components and related classes, unless otherwise documented, must be accessed on the event dispatching thread* – JB Nizet Feb 19 '12 at 18:46
  • So, here's another link from http://docs.oracle.com/javase/tutorial/uiswing/concurrency/dispatch.html (the api links there, so I guess it is not outdated): "Some Swing component methods are labelled "thread safe" in the API specification; these can be safely invoked from any thread." -> No general rule there. – joergl Feb 19 '12 at 18:54