Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Java > Design issue in swing application

Reply
Thread Tools

Design issue in swing application

 
 
Fencer
Guest
Posts: n/a
 
      02-17-2010
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
 
Reply With Quote
 
 
 
 
markspace
Guest
Posts: n/a
 
      02-17-2010
Fencer wrote:

> 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.
 
Reply With Quote
 
 
 
 
Fencer
Guest
Posts: n/a
 
      02-17-2010
On 2010-02-17 16:21, markspace wrote:
> Fencer wrote:
>
>> 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.


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
 
Reply With Quote
 
Lew
Guest
Posts: n/a
 
      02-17-2010
Fencer wrote:
> 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/guidelines/designing_enterprise_applications_2e/web-tier/web-tier5.html>

--
Lew
 
Reply With Quote
 
Fencer
Guest
Posts: n/a
 
      02-17-2010
On 2010-02-17 17:41, Lew wrote:
> Fencer wrote:
>> 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.


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

 
Reply With Quote
 
Lew
Guest
Posts: n/a
 
      02-17-2010
Fencer wrote:
> On 2010-02-17 17:41, Lew wrote:
>> Fencer wrote:
>>> 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.

>
> 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
 
Reply With Quote
 
Lew
Guest
Posts: n/a
 
      02-17-2010
Fencer wrote:
> 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.

--
Lew
 
Reply With Quote
 
markspace
Guest
Posts: n/a
 
      02-17-2010
Fencer wrote:

> 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.
 
Reply With Quote
 
Fencer
Guest
Posts: n/a
 
      02-17-2010
On 2010-02-17 18:52, markspace wrote:
> Fencer wrote:
>
>> 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.


Thank you for posting that!
 
Reply With Quote
 
Fencer
Guest
Posts: n/a
 
      02-17-2010
On 2010-02-17 18:48, Lew wrote:
> Fencer wrote:
>> 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.


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.

>
> 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.
>


 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Long switch statements (and Swing application structural design) RedGrittyBrick Java 2 10-11-2008 03:37 PM
javax.swing.Popup, javax.swing.PopupFactory lizard Java 0 01-30-2006 09:34 PM
Swing Model Classes Updating Swing Components on a Thread Other Than AWT mkrause Java 0 05-06-2005 04:32 PM
Java 1.2 Swing vs. Java 1.5 Swing Big Daddy Java 2 04-16-2005 01:14 PM
Design Issue: Separating Application Security Model from the Application (Custom or User) Controls Earl Teigrob ASP .Net 3 06-10-2004 01:56 AM



Advertisments