Design issue in swing application

[Fencer]

Hello, in my program I toggle between two panels depending on which
phase the program is currently in. I wrote a class that functions as the
entry point of the program and it's composed of a JFrame along with the
components of that frame. Now I wanted to break out my two panels to
make the code more readable so I created two new classes that extend
from JPanel and here's where my design issue arose. Consider the code
below (people who have been following my other thread will recognise it
;-)) which is for the "welcome panel" that is shown when the program is
launched:

package gui;

import java.awt.Dimension;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JPanel;
import javax.swing.border.TitledBorder;

public class WelcomePanel extends JPanel {

private static final long serialVersionUID = 1596841645688614873L;

public WelcomePanel(final CenteredGroup inst) {
this.inst = inst;

setPreferredSize(new Dimension(480, 100));
setBorder(new TitledBorder("Start a new session by opening a
BioModel or load a previously saved session"));
setLayout(new GridBagLayout());

initButtons();
}

private void initButtons() {
final CenteredGroup inst2 = this.inst;
b1 = new JButton("Open BioModel");

b1.addActionListener(new ActionListener() {
@Override public void actionPerformed(ActionEvent e) {
inst2.eventOpenBioModel();
}});

b2 = new JButton("Load Saved Session");

b2.addActionListener(new ActionListener() {
@Override public void actionPerformed(ActionEvent e) {
inst2.eventLoadSavedSession();
}});

addButtonsToGroupPanel(b1, b2);
}

private void addButtonsToGroupPanel(JButton b1, JButton b2) {
GridBagConstraints gbc = new GridBagConstraints();

gbc.gridx = 0;
gbc.gridy = 0;
gbc.gridwidth = 1;
gbc.gridheight = 1;
gbc.weightx = 0.5;
gbc.weighty = 0;
gbc.anchor = GridBagConstraints.CENTER;
gbc.fill = GridBagConstraints.NONE;

add(b1, gbc);

gbc.gridx = 1;

add(b2, gbc);
}

private CenteredGroup inst = null;

private JButton b1 = null;
private JButton b2 = null;
}

What I don't like here are the event handlers for the buttons and, more
specifically, I don't like these two things:
1. The class WelcomePanel knows about its "parent", CenteredGroup (I
will change that name btw).
2. I had to change the event*-methods in CenteredGroup from being
private to default access level so I could call them.

Am I right to worry about these design issues and, if so, how should I
solve them? Maybe I shouldn't have created this class in the first place
but I thought the code for CenteredGroup was becoming a bit long.

Thanks for reading and thanks in advance for any help.

- F

 

[markspace]

1. The class WelcomePanel knows about its "parent", CenteredGroup (I
will change that name btw).

Normally, this is not an issue. Your code does appear a bit odd
however, though I haven't pinned down why.

2. I had to change the event*-methods in CenteredGroup from being
private to default access level so I could call them.

Not a problem at all. Normally, you code to interfaces, which are
nearly always public.

Thus CenteredGroup should implement some interface, which other classes
can code too. WelcomePanel can use that public interface to call
CenteredGroup's methods.

I think what's weird about your code is the lack of MVC. The code seems
all sort of muddled together to me. Take a look at your problem, break
it into MVC, and add interfaces for the views and the model (since this
is just welcome screen, you may have very little "model" in your code).
Then see if you can explain it in a higher level context.

 

[Fencer]

Normally, this is not an issue. Your code does appear a bit odd however,
though I haven't pinned down why.




Not a problem at all. Normally, you code to interfaces, which are nearly
always public.

Thus CenteredGroup should implement some interface, which other classes
can code too. WelcomePanel can use that public interface to call
CenteredGroup's methods.

I think what's weird about your code is the lack of MVC. The code seems
all sort of muddled together to me. Take a look at your problem, break
it into MVC, and add interfaces for the views and the model (since this
is just welcome screen, you may have very little "model" in your code).
Then see if you can explain it in a higher level context.

Thanks for your reply.
MVC, hmm. Ok, so imagine WelcomePanel is a view and instead of knowing
about the CenteredGroup class directly it knows about an interface for a
Controller. When the user presses the button "Open BioModel" the view
informs the controller about this. What should happen now is that a file
selection dialog should appear where the user can select a biomodel file
which is then processed. But who should display this file selection dialog?
Maybe WelcomePanel should do that and then inform the controller that
this event has occurred and this biomodel file was choosen and the
controller would then proceed from there?

As you can see I have trouble deciding what goes where, heh.

- Fencer

 

[Lew]

MVC, hmm. Ok, so imagine WelcomePanel is a view and instead of knowing
about the CenteredGroup class directly it knows about an interface for a Controller.

Normally you'd do it the other way around - the controller knows about the
view, but the view doesn't know about the controller.

When the user presses the button "Open BioModel" the view
informs the controller about this. What should happen now is that a file

This is where the controller knows the view. The central view method returns
a value, and takes arguments, that the controller understands. The view types
know nothing about how the arguments are created or the return values are used.

selection dialog should appear where the user can select a biomodel file
which is then processed. But who should display this file selection dialog?

The view under the direction of the controller.

Maybe WelcomePanel should do that and then inform the controller that

Calling it a welcome "panel" already locks down implementation too far. The
fact that it's a panel is almost certainly irrelevant to the controller.

this event has occurred and this biomodel file was choosen and the
controller would then proceed from there?

The controller knows when to ask for the "Welcome Interaction" (or "Screen" or
whatever application-domain concept you use), what to pass to it, and what to
do with the result, i.e., to where to navigate based on the return value and
what data to pass to the model or behaviors to invoke on it.

References (be sure to follow links from these as well):
<http://en.wikipedia.org/wiki/Model_2>
<http://en.wikipedia.org/wiki/Model-View-Controller>
<http://www.javaworld.com/javaworld/jw-12-1999/jw-12-ssj-jspmvc.html>
<http://java.sun.com/blueprints/guid...prise_applications_2e/web-tier/web-tier5.html>

 

[Fencer]

Normally you'd do it the other way around - the controller knows about
the view, but the view doesn't know about the controller.


This is where the controller knows the view. The central view method
returns a value, and takes arguments, that the controller understands.
The view types know nothing about how the arguments are created or the
return values are used.

Thanks for your reply.

I will read some more and try to make a concept implementation. Just one
question, if the view (a JPanel in my case) doesn't know about the
controller but the controller knows about the view, how should I
implement my button listener? Right now it's an anonymous inner class to
the JPanel-extended class, I don't get how to handle that and not have
the view know the controller?

[snip]

- F

 

[Lew]

Thanks for your reply.

I will read some more and try to make a concept implementation. Just one
question, if the view (a JPanel in my case) doesn't know about the
controller but the controller knows about the view, how should I
implement my button listener? Right now it's an anonymous inner class to
the JPanel-extended class, I don't get how to handle that and not have
the view know the controller?

The listener does exist in the view because it's part of the view. If you
actually read the links to which I pointed, you will see that ultimately the
job of the view is to return state to the controller; you can do that from the
listener or from some other part of the view triggered by the listener.

Read the links, explore their transitive links, think about what you've read,
play with it for a while, then ask questions, in that order. If you haven't
even assimilated the information already provided, then you're going to ask
redundant questions. Read, study, experiment, then ask.

 

[Lew]

package gui;

import java.awt.Dimension;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JPanel;
import javax.swing.border.TitledBorder;

public class WelcomePanel extends JPanel {

private static final long serialVersionUID = 1596841645688614873L;

public WelcomePanel(final CenteredGroup inst) {
this.inst = inst;

setPreferredSize(new Dimension(480, 100));
setBorder(new TitledBorder("Start a new session by opening a
BioModel or load a previously saved session"));
setLayout(new GridBagLayout());

initButtons();
}

private void initButtons() {
final CenteredGroup inst2 = this.inst;

^
You don't need this line.

b1 = new JButton("Open BioModel");

b1.addActionListener(new ActionListener() {
@Override public void actionPerformed(ActionEvent e) {
inst2.eventOpenBioModel();
}});

b2 = new JButton("Load Saved Session");

b2.addActionListener(new ActionListener() {
@Override public void actionPerformed(ActionEvent e) {
inst2.eventLoadSavedSession();
}});

addButtonsToGroupPanel(b1, b2);
}

private void addButtonsToGroupPanel(JButton b1, JButton b2) {
GridBagConstraints gbc = new GridBagConstraints();

gbc.gridx = 0;
gbc.gridy = 0;
gbc.gridwidth = 1;
gbc.gridheight = 1;
gbc.weightx = 0.5;
gbc.weighty = 0;
gbc.anchor = GridBagConstraints.CENTER;
gbc.fill = GridBagConstraints.NONE;

add(b1, gbc);

gbc.gridx = 1;

add(b2, gbc);
}

private CenteredGroup inst = null;

private JButton b1 = null;
private JButton b2 = null;
}

Why do you initialize these instance members to 'null' when a) the language
already does that anyway, and b) you initialize them to non-'null' values in
your constructor anyway?

Just curious.

It's a matter of style, but personally I find placement of member variable
declarations at the top to be clearer than at the bottom. For example, in
this code it would make it more obvious that you are assign values to those
instance variables three times apiece.

For this particular class, where the 'CenteredGroup' and the two 'JButton's
each need exactly one value, you should declare those variables 'final' and
assign their 'new' values in the constructor rather than a separate method.

 

[markspace]

Thanks for your reply.
MVC, hmm. Ok, so imagine WelcomePanel is a view and instead of knowing
about the CenteredGroup class directly it knows about an interface for a
Controller. When the user presses the button "Open BioModel" the view
informs the controller about this. What should happen now is that a file
selection dialog should appear where the user can select a biomodel file
which is then processed. But who should display this file selection dialog?
Maybe WelcomePanel should do that and then inform the controller that
this event has occurred and this biomodel file was choosen and the
controller would then proceed from there?

Lew gave some good links and advice. Here's the same stuff in a
slightly different format.

Normally, the Controller has references to the View and the Model, not
the other way around.

public class Controller {

View view;
Model model;

public Controller ( View view, Model model ) {
this.view = view;
this.model = model;
...
}
....
}

View would be some interface that this controller knows how to deal
with. Your View would be named "WelcomeScreen" or something like that;
this controller might be called a "WelcomeController". The
implementation of the view would be a JPanel or JFrame but your
controller wouldn't care about that.

The thing that receives the user interaction is the controller too. The
view only knows about the controller via a call back, so that needs to
be set.

public Controller ( View view, Model model ) {
this.view = view;
this.model = model;

this.view.addLoadFileActionListener( new ActionListener() {
@Override
public actionPerformed( final ActionEvent e ) {
loadFile( e );
}
}
...
}

loadFile( ActionEvent e ) {
... show JFileDialog here...
}

Something like that. (Note this controller now likely needs to be
synchronized and created on the EDT.)

The result is that your Controller is very procedural. It's a "driver
object". It mostly contains methods that drive the operation of your
program, it doesn't contain a lot of "state." This makes testing
easier. Try to keep controllers specific to one task or idea. If
needed, you should only have one action listener per controller. (More
are OK, if the concept is tightly scoped.) You may need lots of
controllers to implement a single window if that window has lots of
buttons or controls.

Views should be "dumb" and just fire events. Don't keep any state in a
view, other than the state it already has (like the text string in a
JTextField).

Model objects span the gamut. Some are "dumb," just data, like value
objects. Some are more active, and will do things like persists (save)
data, write to databases, or download from URLs or whatnot. It's up to
your design and your requirements how you do the model. Your model
would probably be a "BioModel".

In classic MVC, the model updates the view via a call back, and the view
has a reference to the model. However, Java uses a "split model" design
where views (JComponents) have their own model. So I think it's kind of
a toss up how your model updates the view when needed. I'd be inclined
to have it go through the Controller too, meaning no direct linkage.

In Java it's also possible to dispense with the Controller entirely and
just use the ActionLisenter. However, it's still a useful concept
sometimes, and you should at least keep it in mind for when it's useful.

That's my two minute lesson on MVC. Hopefully some of it is is even
correct.

As you can see I have trouble deciding what goes where, heh.

That's normal in software development.

 

[Fencer]

Lew gave some good links and advice. Here's the same stuff in a slightly
different format.

Normally, the Controller has references to the View and the Model, not
the other way around.

public class Controller {

View view;
Model model;

public Controller ( View view, Model model ) {
this.view = view;
this.model = model;
...
}
...
}

View would be some interface that this controller knows how to deal
with. Your View would be named "WelcomeScreen" or something like that;
this controller might be called a "WelcomeController". The
implementation of the view would be a JPanel or JFrame but your
controller wouldn't care about that.

The thing that receives the user interaction is the controller too. The
view only knows about the controller via a call back, so that needs to
be set.

public Controller ( View view, Model model ) {
this.view = view;
this.model = model;

this.view.addLoadFileActionListener( new ActionListener() {
@Override
public actionPerformed( final ActionEvent e ) {
loadFile( e );
}
}
...
}

loadFile( ActionEvent e ) {
... show JFileDialog here...
}

Something like that. (Note this controller now likely needs to be
synchronized and created on the EDT.)

The result is that your Controller is very procedural. It's a "driver
object". It mostly contains methods that drive the operation of your
program, it doesn't contain a lot of "state." This makes testing easier.
Try to keep controllers specific to one task or idea. If needed, you
should only have one action listener per controller. (More are OK, if
the concept is tightly scoped.) You may need lots of controllers to
implement a single window if that window has lots of buttons or controls.

Views should be "dumb" and just fire events. Don't keep any state in a
view, other than the state it already has (like the text string in a
JTextField).

Model objects span the gamut. Some are "dumb," just data, like value
objects. Some are more active, and will do things like persists (save)
data, write to databases, or download from URLs or whatnot. It's up to
your design and your requirements how you do the model. Your model would
probably be a "BioModel".

In classic MVC, the model updates the view via a call back, and the view
has a reference to the model. However, Java uses a "split model" design
where views (JComponents) have their own model. So I think it's kind of
a toss up how your model updates the view when needed. I'd be inclined
to have it go through the Controller too, meaning no direct linkage.

In Java it's also possible to dispense with the Controller entirely and
just use the ActionLisenter. However, it's still a useful concept
sometimes, and you should at least keep it in mind for when it's useful.

That's my two minute lesson on MVC. Hopefully some of it is is even
correct.



That's normal in software development.

Thank you for posting that!

 

[Fencer]

^
You don't need this line.


Why do you initialize these instance members to 'null' when a) the
language already does that anyway, and b) you initialize them to
non-'null' values in your constructor anyway?

Just curious.

It's a matter of style, but personally I find placement of member
variable declarations at the top to be clearer than at the bottom. For
example, in this code it would make it more obvious that you are assign
values to those instance variables three times apiece.

Ah, I learned something useful there! I usually initialize to null even
though that is done automatically but now you taught me that if I dont
initalize to null I can use final! Thank you.

 

[Lew]

Normally, the Controller has references to the View and the Model, not
the other way around.

public class Controller {

View view;
Model model;

More commonly the controller could have a collection of model classes or
instances and map of {model instance, outcome} -> {view instance} entries.

There would also be a map of {view result} -> {model instance} entries.

A framework like Struts or JSF can build the maps at deployment time.

(Note this controller now likely needs to be
synchronized and created on the EDT.)

The controller should definitely not run on the EDT. It or (preferably) the
view components it invokes should use 'invokeAndWait()'.

I agree with the bulk of your comments, however.

You may need lots of
controllers to implement a single window if that window has lots of
buttons or controls.

There are various MVC paradigms. The simple "Model 2" version (for example,
Struts) has a single front controller. More fractal versions such as JSF do
what you suggest, having multiple controllers.

Views should be "dumb" and just fire events. Don't keep any state in a
view, other than the state it already has (like the text string in a
JTextField).

The view may also handle surface edits and other view logic to ensure that
data that reach the controller(s) from the view are consistent.

In classic MVC, the model updates the view via a call back, and the view
has a reference to the model. However, Java uses a "split model" design

I consider "classic" the MVC paradigm that is, as you say, procedural and the
controller loop looks something like:

public void control()
{
for ( View view = initialView(); view != BYEBYE
{
Request request = view.getRequest();
Model model = chooseModel( request );
Result result = model.process( request );
view = nextView( model, result );
}
}

where 'chooseModel()' and 'nextView()' are methods in the controller and
'View' and 'Model' are interfaces with a 'getRequest()' and 'process()'
method, respectively.

When I've written Model 2 code by hand, the controller is only a couple of
hundred lines long, and most of that comprises the setup code for the maps.

where views (JComponents) have their own model. So I think it's kind of
a toss up how your model updates the view when needed. I'd be inclined
to have it go through the Controller too, meaning no direct linkage.

Exactly so.

 

[markspace]

markspace wrote:

public Controller ( View view, Model model ) {
this.view = view;
this.model = model;

this.view.addLoadFileActionListener( new ActionListener() {
@Override
public actionPerformed( final ActionEvent e ) {
loadFile( e );
}
}

The controller should definitely not run on the EDT. It or (preferably)
the view components it invokes should use 'invokeAndWait()'.

What I was looking at there was a this-escape. The anonymous
ActionListener I create there has an implicit this pointer that is
passed to the view, probably a JComponent, which can then fire events
asynchronously. Creating the Controller by calling the constructor on
the EDT would alleviate the asynchronous calling part, although the
this-escape still isn't great. It's basically a coding boo-boo I didn't
have time to fix.

Regardless of the this-escape, I think actionListeners (or their
equivalents) should be created and installed on the EDT, to prevent them
from firing asynchronously before the rest of the GUI is built.

I consider "classic" the MVC paradigm that is, as you say, procedural
and the controller loop looks something like:

Pretty much everything I know about other MVC patterns comes from Martin
Fowler's white paper on the subject:

<http://www.martinfowler.com/eaaDev/uiArchs.html>

 

[Lew]

Regardless of the this-escape, I think actionListeners (or their
equivalents) should be created and installed on the EDT, to prevent them
from firing asynchronously before the rest of the GUI is built.

Yes, absolutely.

A proper Controller will not define the listeners per se, but pick up the
result (what I called a "Request" in my mini-example upthread) from a view
component (such as Strut's 'ActionForm') method call. For a Swing view, that
view component will handle the inter-thread communication so that any
listeners will live on the EDT but the view component will return a request to
the controller off the EDT.

If the controller is aware of Swing, it's not proper separation between the
view and the controller.

 

[Fencer]

Lew gave some good links and advice. Here's the same stuff in a slightly
different format.

Normally, the Controller has references to the View and the Model, not
the other way around.

public class Controller {

View view;
Model model;

public Controller ( View view, Model model ) {
this.view = view;
this.model = model;
...
}
...
}

View would be some interface that this controller knows how to deal
with. Your View would be named "WelcomeScreen" or something like that;
this controller might be called a "WelcomeController". The
implementation of the view would be a JPanel or JFrame but your
controller wouldn't care about that.

The thing that receives the user interaction is the controller too. The
view only knows about the controller via a call back, so that needs to
be set.

public Controller ( View view, Model model ) {
this.view = view;
this.model = model;

this.view.addLoadFileActionListener( new ActionListener() {
@Override
public actionPerformed( final ActionEvent e ) {
loadFile( e );
}
}
...
}

loadFile( ActionEvent e ) {
... show JFileDialog here...

Based on this code, I wrote a simple application pasted below. It
doesn't actually have a model because I wanted to keep things as short
as possible. Also, my controller knows about Swing in this
implementation, just wanted to say that right away. It's not going to be
perfect but I hope to have my application to incorporate some of the
spirit behind MVC at least.

Something that worries me with the code below is that the class holding
the frame, AppEntry, which also serves as the entry point of the
application, isn't a view itself. Instead it creates my single view and
a single controller. Say I add a menu bar and status bar (a JLabel) to
the frame, those would also be views known about some controller?

The application I am attempting to write is a bioinformatics application
where I envision having a menu, a status bar, a welcome view when the
program isn't not working on an bioinformatic model and a data view when
the program is indeed working with a bioinformatic model and I haven't
figured out how to incorporate that into MVC.

Say a user starts the application and is greeted by the welcome view.
The user wants to load a biomodel so he or she clicks the corresponding
button. In my case the controller knows about swing and it detects the
button click through its anonymous inner ActionListener class. The
controller shows a file picker allowing the user to select the biomodel
he or she desires. This biomodel is passed to the model which does a lot
of processing on it to build a tree structure. I won't go into details
but this is by far the most complicated thing I will have in my
application. This tree structure is supposed to be displayed by another
view, let's call it the data view. So my model needs to pass display
data in a format that the data view can understand and then tell my
AppEntry class, which holds the frame, to create a new data view and
show that instead of the welcome view. All this through some controller
somehow. I'm not sure how to build this.

/* AppEntry.java start */
package main;

import java.awt.EventQueue;

import javax.swing.JFrame;

public class AppEntry {

public AppEntry() {
frame = new JFrame("MVC attempt");

frame.setSize(640, 480);
frame.setLocationRelativeTo(null);
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

welcomePanel = new WelcomeImpl();

frame.add(welcomePanel);

controller = new Controller(welcomePanel);
}

public void show() {
frame.setVisible(true);
}

public static void main(String[] args) {
EventQueue.invokeLater(new Runnable() {
public void run() {
new AppEntry().show();
}
});

}

private final JFrame frame;
private final WelcomeImpl welcomePanel;
private final Controller controller;
}
/* AppEntry.java end */

/* Welcome.java start */
package main;

import java.awt.event.ActionListener;

public interface Welcome {
void addB1ActionListener(ActionListener actionListener);
}
/* Welcome.java end */

/* WelcomeImpl.java start */
package main;

import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JPanel;

public class WelcomeImpl extends JPanel implements Welcome {

private static final long serialVersionUID = -8645551859827091979L;

public WelcomeImpl() {
b1 = new JButton("b1");

add(b1);
}

public void addB1ActionListener(ActionListener actionListener) {
b1.addActionListener(actionListener);
}

private final JButton b1;
}
/* WelcomeImpl.java end */

/* Controller.java start */
package main;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class Controller {
public Controller(Welcome welcomeView) {
this.welcomeView = welcomeView;

this.welcomeView.addB1ActionListener(new ActionListener() {
@Override public void actionPerformed(ActionEvent arg0) {
System.out.println("Show file selection dialog, send
file to model?");
}});
}

private Welcome welcomeView;
}
/* Controller.java end */

It's probably crap and I probably haven't read enough, but as I said I
want to incorporate some of the ideas of MVC into this application. I
want to design it as best as I can but I don't have time to make it
perfect and its actual functionality, what the user sees, that's
complicated for me to write so I have to spend most of my time on that.

PS. The files my program operate on are called BioModels, not to be
confused with Model in MVC.

- F

 

[Daniel Pitts]

Yes, absolutely.

A proper Controller will not define the listeners per se, but pick up
the result (what I called a "Request" in my mini-example upthread) from
a view component (such as Strut's 'ActionForm') method call. For a Swing
view, that view component will handle the inter-thread communication so
that any listeners will live on the EDT but the view component will
return a request to the controller off the EDT.

If the controller is aware of Swing, it's not proper separation between
the view and the controller.

I think you've topsy-turvied your concepts here. Controllers know the
details of what it is controlling. In a pure design:

The Controller would know whether to put something on the EDT or not.
The View doesn't know about specific controllers or models.
The Model doesn't know about sepcific controllers or views.
The Controller "connects" the view and model to each-other through
generic interfaces.

 

[RedGrittyBrick]

I will read some more and try to make a concept implementation. Just one
question, if the view (a JPanel in my case) doesn't know about the
controller but the controller knows about the view, how should I
implement my button listener? Right now it's an anonymous inner class to
the JPanel-extended class, I don't get how to handle that and not have
the view know the controller?

There is no single correct way to implement MVC. This may not be right -
it's just answering your question as stated and in isolation.

interface ListenableView {
void addListener(Listener l);
...
}

class FooView implements ListenableView {
...
void addListener(Listener listener) {
fooButton.addListener(listener);
}
}

class BarController implements Listener {
...
someMethod(...) {
...
aListenableView.addListener(this)
}
}