Have you guys seen this pattern before? If so, do you see it a lot?

First a notional Application class (most obvious stuff omitted 'cuz I'm lazy):

public class Application {

   public DAO dao = null;

   public Application() {
     dao = new DAO();
   }

   public void createAndShowGUI() {
     // do stuff to display the GUI
   }

   public static void main(String[ args) {
     Application app = new Application();
     app.createAndShowGUI();
   }

   public void newWindow(Container c) {
     MyFrame w = createWindow(c, this);
     w.setVisible(true);
   } 

   public MyComponent createWindow(Container c, Application app) {
     MyFrame w = new MyFrame(app);
     w.setContentPane(c);
     w.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
     return w;     
   }
}

Take special note of the public access DAO ...

Now, the child window class (again, obvious stuff omitted):

public class MyFrame extends JFrame {

  private Application app = null;

  public MyFrame(Application app) {
    this.app = app;
  }

  public void setContentPane(Component c) {
    super.setContentPane(c);
    this.pack();
  }     
 
  private void doSomething() {
    Criteria blah = getDataFromForm();
    Results results = app.dao.getDataFromDatabaseBasedOn(blah);
    updateGUI(results);
  }
}

This is the way our primary business app GUI is written. When someone clicks on the button labelled "Do Something", doSomething() is called through an ActionListener on that button. The doSomething() method then uses the "global" DAO accessed through the back-reference to the Application class. There are 137 of these back-references to Application throughout our code whenever someone needs to use the DAO.

I consider this very WTFy ...

I know exactly what you're talking about, and I find it WTFy. I have a large GWT application that passes around an ApplicationBundle class to pretty much every single class you can imagine.
I've started using the singleton pattern here, so that I can do stuff like the following.
ApplicationBundle.getInstance().getThingDao.getById(id);
I find this easier to manage than a bunch of constructors passing around ApplicationBundle all over the place.

This can also be mitigated by using a dependency injection framework such as spring or GIN. Then you just code to interfaces on any class that uses ThingDAO, and inject an instance at runtime.

zelmak:

When someone clicks on the button labelled "Do Something", doSomething() is called through an ActionListener on that button. The doSomething() method then uses the "global" DAO accessed through the back-reference to the Application class.

Actually, I lied about this. Just about every ActionListener that isn't trivial action (such as "show help box") is coded as an anonymous inner class (similar to a closure in other languages which support first-class function objects). Most are several 10's to 100's of lines long.

JButton doSomethingButton = new JButton("Do Something");
doSomethingButton.addActionListener(new ActionListener() {
  public void actionPerformed(ActionEvent evt) {
    // code to access database
    // manipulate and filter results ('cuz we can't do that in the query for some reason)
    // code to update the UI, usually a JTable with a DefaultTableModel
  } 
});

They love their grids around here... Some days, I want show them how to connect Excel to the Oracle DB and tell them to have fun...

Shouldn't the UI be communicating to a business layer, not directly to the DAL?

dependency injection framework such as spring or GIN.

I'd suggest Google Guice.

Yeah, the person who started (architected?) this application didn't know about MVC, encapsulation and I doubt OOP in general

ubersoldat:

Yeah, the person who started (architected?) this application didn't know about MVC

I think they heard of it, but they thought it was all one thing.

ubersoldat:

didn't know about MVC, encapsulation and I doubt OOP in general

 

Well, it's not like they're concepts important to systems architecture now, are they?

Oh, wait --

I don't consider this so much "WTFy".

When deciding whether a design a good or bad I'd say use these criteria:

1. Does it require lots of redundant code/boilerplate? (This is a problem because it slows down development)

2. Would a single change in the requirements require lots of changes in many different places? (This is a problem because one may forget to change one of these places when implementing such a change)

3. Is there a risk a future developer misuses the design and thereby creates bugs?

4. Is it a well-known pattern? (If not, it is a problem because new developers have to learn the pattern first)

 

Now let's analyse this design.

1. The redundant code is the app passed to every single application element constructor, and the fact that all dao accesses need to be written app.dao instead of just dao. I'd say this is pretty minor. Note that using a singleton pattern "Application.getInstance().getDao()"is not that much shorter especially as you now need to write the implementation of getDao().

2.  Not really. You want to change the dao implementation by another one? Only modify Application. You find out MyFrame has another dependency (in addition to dao)? Just access it through application - no need to modify Application or whatever code instanciates MyFrame.

3. There are two risks here: first Application.dao is writable, so a developer might be tempted to *modify* Application.dao from another place (which is the reason people say globals are "bad" - because they can be modified from a method without the method signature making this evident). This could be avoided by marking dao as final in application, and adding an Application constructor to pass custom dao implementations (for use from e.g. unit tests). Second risk: there's a Singleton pattern at use here (people assume there's only one application instance so that everytime someone writes app.dao they access the dao of the same app, and therefore the same dao as well), BUT people might be tempted to pass a new Application instance. Solution: mark Application's constructors as private (or maybe package private), [OR mark dependencies (dao and others) as static - in that case you no longer need to pass the application instance. This second solution is less good because it prevents you from making unit tests with different dao implementations AND still having dao marked final.]

 4. This is not the most usual way of doing dependency injection, which is probably why the first reaction is WTF. But it doesn't IMO have issues itself.

 (Using a dao directly instead of having a service layer is another issue that I did not cover above. That MAY be a problem if data has/may have in future integrity/consistency requirements that would need to be implemented in a service, but it is orthogonal to my analysis above - nothing would prevent Application having a public Service attribute, and have the Service itself use a Dao).

 

(PS I accessed this account through bugmenot - treat this as slashdot's anonymous coward ; I did not write or even read any of Strolskon's other posts)

Strolskon:

2. Would a single change in the requirements require lots of changes in many different places?

 

Loose coupling techniques and modular design approaches prevent interdependencies ("action at a distance") like this -  found in things like MVC.

Strolskon:

3. Is there a risk a future developer misuses the design and thereby creates bugs?

This will always be a risk. The risk can be reduced by raising awareness (including discussing benefits of correct us and impact of improper use) as well as implementing preventative measures but it's down to the organisation to decide the effort required. I believe it's a good investment, but many managers may not see it that way.

Having a reference back up to the top level Application isn't the worst I've seen - that honour belongs to each object having a reference to its immediate parent in the hierarchy,and then finding someone coding a turd like this.Parent.Parent.Parent.Parent.Parent.Dao.GetFoo(), thereby cementing the entire object hierarchy in place for all time. But the whole idea of objects knowing anything about the innards of their callers basically kills the idea of code reuse stone dead. Leading to the obvious pile of copypasta down the line...

I've seen this, or rather, I've done this in university.

We were programming a game with C#/XNA, and were told to pass through an instance of the "Game" object to every object we create that needed reference to any other game entities.

In their defence though, this course was not meant to teach good coding practises (We had several courses on why this is wrong before this one), but to show us loosely what is behind making a game (Or more accurately, to point out that a lot of it ISNT coding).

Perhaps the point was to inadvertedly point out what sort of code this leads to in a group environment? I don't know.

darkmatter:

I know exactly what you're talking about, and I find it WTFy. I have a large GWT application that passes around an ApplicationBundle class to pretty much every single class you can imagine.
I've started using the singleton pattern here, so that I can do stuff like the following.
ApplicationBundle.getInstance().getThingDao.getById(id);
I find this easier to manage than a bunch of constructors passing around ApplicationBundle all over the place.

That's from bad to worse. Singleton is even better equivalent of global variable than passing around big-bag-of-instances.

Yes, I learned that the hard way, why do you ask? (We have an application that passed App instance everywhere, so we replaced it with singletons and it's even worse spaghetti now and since I read Singletons: Solving problems you didn’t know you never had since 1995 I even understand why.

 My very first Java application (other than "Hello, world!" and such) was written in a very similar manner. The main reason, other than unfamiliarity? I followed Sun's examples on how to write Swing applications. They're all like this, using inheritance rather than composition, etc.

I get blinded by WTF every time I see this naming convention:

zelmak:

public DAO dao = null;

 

Bulb:

That's from bad to worse. Singleton is even better equivalent of global variable than passing around big-bag-of-instances.

Yes, I learned that the hard way, why do you ask? (We have an application that passed App instance everywhere, so we replaced it with singletons and it's even worse spaghetti now and since I read Singletons: Solving problems you didn’t know you never had since 1995 I even understand why.

It's the biggest antipattern out there. At least a god object isn't actually claiming to follow a pattern, it's just a big heap of stuff whereas the structural singleton actually makes any code that uses it worse.

 

JimLahey:

Bulb:

That's from bad to worse. Singleton is even better equivalent of global variable than passing around big-bag-of-instances.

Yes, I learned that the hard way, why do you ask? (We have an application that passed App instance everywhere, so we replaced it with singletons and it's even worse spaghetti now and since I read Singletons: Solving problems you didn’t know you never had since 1995 I even understand why.

It's the biggest antipattern out there. At least a god object isn't actually claiming to follow a pattern, it's just a big heap of stuff whereas the structural singleton actually makes any code that uses it worse.

 

Singletons can be very useful when used correctly, especially when used to access scarce/expensive resources. The problem is, like any pattern, it's overused, especially as a substitute for global variables.

I'd also like to correct a point that blog article makes: 'it guar­an­tees global access to this one instance' if you give it public accessibility. If you give it protected/package visibility, then it's not exactly a global, is it?

BTW: protected/private static/final (pick and choose based on language) is just the singleton in its simplest form.

 

zelmak:

This is the way our primary business app GUI is written. When someone clicks on the button labelled "Do Something", doSomething() is called through an ActionListener on that button. The doSomething() method then uses the "global" DAO accessed through the back-reference to the Application class. There are 137 of these back-references to Application throughout our code whenever someone needs to use the DAO.

 How to call it? As already mentioned, this is very likely a "god object" or "god class".

 And it definitely is a violation of the Law of Demeter!

ubersoldat:

Yeah, the person who started (architected?) this application didn't know about MVC, encapsulation and I doubt OOP in general

When I see statements like that, what I perceive is that their author has never had to do anything really critical, or work with limited resources. OOP is the discredited and stifling orthodoxy of the 1990s. MVC is an ill-defined attempt to fix the massive human factors issues associated with OOP by artificially constraining OOP to a less destructive role. None of this is to say that the "application global" pattern you identified is ideal. Properly implemented, though, it's vastly superior to a lot of the crap you and others are suggesting in this thread.

10/10 you win the thread.

 I want a pattern that can reasonably be called "Penrose Tiling".

It looks like this:

Along with "Our Roj" himself.

(he's not actually standing on it - he's affixed to the floor on his back and only appears to be standing upright)