Could you comment on my little program? Thank you!

S

Shawn

I have finished my little gui program. Code is pasted below. Two things
I feel proud about my code:

(1)I wrote one class JMenuPower, subclass of JMenu. I can put objects of
JMenuItem, which will 1)automatically register ActionListener to "this"
object 2)setActionCommand to the String parameter passed in 3)put both
the actionCommand string and the Mapper reference into a HashMap for
later retrieval.

In conclusion, my JMenuPower class is in higher level than JMenu class.
JMenu class is like primitive class for me now.
So my brain can have less burden.

(2)When user clicks the different Menu Item, the corresponding operation
is evoked dynamically. I achieved this goal by using an interface
Mapper. So if I need to add one Menu Item, I only need to do two things:

a)
MenuItem m = new JMenuItem("new one");
memoMenu.addMenuItemAndListener(m, "newone", newoneMapper,
this); //throw it into my JMenuPower object, which takes care of the rest

b) //write the Maper do what I want to do
Mapper newoneMapper = new Mapper()
{
public void menuItemAction()
{
//put code here
}
};

Do you think I am twisting too much or I am doing correct thing?

Thank you very much for your feedback.

===============JMenuPower.java==============
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.util.*;

/****************************************************************************************
* My class extends from JMenu: the objects of JMenuItem can be added
into the object of
* this class. It will automatically register the actionlistener so the
lines in the client
* class is reduced and concept is clearer: add the menu item and
register the action listener.
* In another words, JMenuPower class is higher level than JMenu class now.
*
*/
public class JMenuPower extends JMenu
{
private Map<String, Mapper> myActions = new HashMap<String,
Mapper>(); //hold the String and its corresponding Mapper (different
action)

public JMenuPower(String s)
{
super(s);
}

void addMenuItemAndListener(JMenuItem mi, ActionListener al)
{
mi.addActionListener(al);
this.add(mi);
}

void addMenuItemAndListener(JMenuItem mi, String str, Mapper
mapper, ActionListener al)
{
mi.addActionListener(al);
mi.setActionCommand(str);
this.add(mi);

myActions.put(str, mapper);
}

public Mapper getActionMapper(String str)
{
return myActions.get(str);
}

}


=================MemoGui3.java======================
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.lang.reflect.*;

interface Mapper {
public void menuItemAction();
}

public class MemoGUI3 extends JFrame implements ActionListener
{
public static final int WIDTH = 600;
public static final int HEIGHT = 300;
public static final int LINES = 10;
public static final int CHAR_PER_LINE = 40;

private JTextArea theText;
private String memo1 = "No Memo 1";
private String memo2 = "No Memo 2";

JMenuPower memoMenu;

public MemoGUI3()
{
this.setSize(WIDTH, HEIGHT);
this.addWindowListener(new WindowDestroyer());
this.setTitle("Memo Saver");
Container contentPane = this.getContentPane();
contentPane.setLayout(new BorderLayout());

memoMenu = new JMenuPower("Memos"); //my own JMenuPower class
JMenuItem m;

//Just throw the Menu Item object into the object of JMenuPower
class. It will take care of the rest:
m = new JMenuItem("Save Memo 1");
memoMenu.addMenuItemAndListener(m, "save1", save1Mapper, this);

m = new JMenuItem("Save Memo 2");
memoMenu.addMenuItemAndListener(m, "save2", save2Mapper, this);

m = new JMenuItem("Get Memo 1");
memoMenu.addMenuItemAndListener(m, "get1", get1Mapper, this);

m = new JMenuItem("Get Memo 2");
memoMenu.addMenuItemAndListener(m, "get2", get2Mapper, this);

m = new JMenuItem("Clear");
memoMenu.addMenuItemAndListener(m, "clear", clearMapper, this);

m = new JMenuItem("Exit");
memoMenu.addMenuItemAndListener(m, "exit", exitMapper, this);

JMenuBar mBar = new JMenuBar();
mBar.add(memoMenu);
this.setJMenuBar(mBar);

JPanel textPanel = new JPanel();
textPanel.setBackground(Color.blue);
theText = new JTextArea(LINES, CHAR_PER_LINE);
theText.setBackground(Color.white);
textPanel.add(theText);
contentPane.add(textPanel, BorderLayout.CENTER);

}

//this part of code never needs to be changed, even when a new Menu
Item needs to be added. I feel this is great:
public void actionPerformed(ActionEvent e)
{
String actionStr = e.getActionCommand();

(memoMenu.getActionMapper(actionStr)).menuItemAction();
}

//All the mess (the details of what the program should do when user
clicks the Menu Item) is pushed to here, -- the lowest level

//Mapper definitions

Mapper save1Mapper = new Mapper()
{
public void menuItemAction()
{
memo1 = theText.getText();
}
};

Mapper save2Mapper = new Mapper()
{
public void menuItemAction()
{
memo2 = theText.getText();
}
};

Mapper get1Mapper = new Mapper()
{
public void menuItemAction()
{
theText.setText(memo1);
}
};

Mapper get2Mapper = new Mapper()
{
public void menuItemAction()
{
theText.setText(memo2);
}
};

Mapper clearMapper = new Mapper()
{
public void menuItemAction()
{
theText.setText("");
}
};

Mapper exitMapper = new Mapper()
{
public void menuItemAction()
{
System.exit(0);
}
};


//end of Mapper definition block


public static void main(String[] args)
{
MemoGUI3 gui = new MemoGUI3();
gui.setVisible(true);
}

}
 
O

Oliver Wong

Shawn said:
I have finished my little gui program. Code is pasted below. Two things
I feel proud about my code:

(1)I wrote one class JMenuPower, subclass of JMenu. I can put objects of
JMenuItem, which will 1)automatically register ActionListener to "this"
object 2)setActionCommand to the String parameter passed in 3)put both
the actionCommand string and the Mapper reference into a HashMap for
later retrieval.

In conclusion, my JMenuPower class is in higher level than JMenu class.
JMenu class is like primitive class for me now.
So my brain can have less burden.

(2)When user clicks the different Menu Item, the corresponding operation
is evoked dynamically. I achieved this goal by using an interface
Mapper. So if I need to add one Menu Item, I only need to do two things:

a)
MenuItem m = new JMenuItem("new one");
memoMenu.addMenuItemAndListener(m, "newone", newoneMapper,
this); //throw it into my JMenuPower object, which takes care of the rest

b) //write the Maper do what I want to do
Mapper newoneMapper = new Mapper()
{
public void menuItemAction()
{
//put code here
}
};

Do you think I am twisting too much or I am doing correct thing?
Click to expand...
[code snipped]

I don't think you should have the client code pass a string in, because
if the client unwittingly passes in the same string twice, that could screw
things up. Instad, your JMenuPower class should take care of generating a
unique identifier (not nescessarly a String) for every Mapper passed in.

I'd also probably use more anonymous classes. That is, instead of:

<oldCode>
Mapper newoneMapper = new Mapper() {
public void menuItemAction() {
//put code here
}
};
MenuItem m = new JMenuItem("new one");
memoMenu.addMenuItemAndListener(m, newoneMapper, this);
</oldCode>

I'd write:

<newCode>
MenuItem m = new JMenuItem("new one");
memoMenu.addMenuItemAndListener(m,
new Mapper() {
public void menuItemAction() {
//put code here
}
}, this);
</newCode>

Finally, what you've implemented is very similar to the "Command" design
pattern: http://en.wikipedia.org/wiki/Command_pattern

Sun's class library already has some facilities for exploiting the
command design pattern, using the Action interface and AbstractAction class.
To use it, you'd write code like:

<code>
AbstractAction someAction = new AbstractAction("Save to memo 1", iconSave) {
public void actionPerformed(ActionEvent ae) {
//put code here
}
}
MenuItem m = new JMenuItem(someAction);
</code>

- Oliver
 
I

Ingo R. Homann

Hi,

I think, two things are more complicated than necessary:

(1) (I'm repeating myself) You have two 'identical' Objects (Memos) but
you do not store them in an Array, but in two single variables. So, much
of your code is duplicated (save1Mapper/save2Mapper, ...)

(2) There is one level of 'dispatching' more than necessary:
You are using one MenuItemListener which needs to dispatch the action
using your Map<String, Mapper>. Of course you have to fill the Map in
advance.
It would be easier, if you would use several MenuItemListener (instead
of several Mappers) and add these to the different MenuItems.
So, sou could spare the Map and some of the Code.

Ciao,
Ingo
 
S

Shawn

Oliver said:
[code snipped]

I don't think you should have the client code pass a string in,
because if the client unwittingly passes in the same string twice, that
could screw things up. Instad, your JMenuPower class should take care of
generating a unique identifier (not nescessarly a String) for every
Mapper passed in.
Click to expand...

Thank you. Could you elaborate it more? I cannot follow you.
 
O

Oliver Wong

Shawn said:
Oliver said:
[code snipped]

I don't think you should have the client code pass a string in,
because if the client unwittingly passes in the same string twice, that
could screw things up. Instad, your JMenuPower class should take care of
generating a unique identifier (not nescessarly a String) for every
Mapper passed in.
Click to expand...

Thank you. Could you elaborate it more? I cannot follow you.
Click to expand...

Your API looks something like this:

addMenuItemAndListener(MenuItem menuItem, String someUniqueCode, Mapper
actionCode, ActionListener listener)

I'm saying you shouldn't have the code which calls this method provide
the string which is a unique code. For one reason is that they can't be sure
it's really unique. What if you're working on a project with 5 other team
members? You'd have to coordinate which each other some sort of system to
ensure you don't all pick the same unique code.

You can avoid this problem by changing the API to:

addMenuItemAndListener(MenuItem menuItem, Mapper actionCode, ActionListener
listener)

and generating a unique code within the method itself.

- Oliver
 
C

Chris Uppal

Oliver said:
I'm saying you shouldn't have the code which calls this method provide
the string which is a unique code. For one reason is that they can't be
sure it's really unique. What if you're working on a project with 5 other
team members? You'd have to coordinate which each other some sort of
system to ensure you don't all pick the same unique code.
Click to expand...

Although Shawn could also avoid this problem, and simplify the whole code a lot
more, by adopting Ingo's suggestion -- ditch Mappers (they were a good idea but
they've served their purpose, and can now be transcended) and simply turn each
specialised kind of Mapper into a specialised kind of ActionListener. That way
the main window doesn't need to be an ActionListener itself, and doesn't need
explicit code to dispatch the event to whichever Mapper object is named by some
String.

-- chris
 
S

Shawn

Ingo said:
Hi,

I think, two things are more complicated than necessary:

(1) (I'm repeating myself) You have two 'identical' Objects (Memos) but
you do not store them in an Array, but in two single variables. So, much
of your code is duplicated (save1Mapper/save2Mapper, ...)
Click to expand...
First of all, thank you very much for your kind help. I greatly
appreciate it.

I never get it:
<old-code>
private String memo1 = "No Memo 1";
private String memo2 = "No Memo 2";
</old-code>

<new-code>
private String[] = memo{"No Memo 1", "No Memo 2"};
</new-code>

That's it. I cannot see how to spare other lines of code. I still need:

Mapper save1Mapper = new Mapper()
{
public void menuItemAction()
{
memo[0] = theText.getText(); //replace memo1 by memo[0]
}
};

Mapper save2Mapper = new Mapper()
{
public void menuItemAction()
{
memo[1] = theText.getText(); //replace memo2 by memo[1]
}
};
(2) There is one level of 'dispatching' more than necessary:
You are using one MenuItemListener which needs to dispatch the action
using your Map<String, Mapper>. Of course you have to fill the Map in
advance.
It would be easier, if you would use several MenuItemListener (instead
of several Mappers) and add these to the different MenuItems.
So, sou could spare the Map and some of the Code.
Click to expand...

Fully agree. I realized in this situation I don't need the interface
Mapper. There is one already for me to use. That is ActionListener. I
can use several inner classes (or ananymous classes directly in the
parameter) implementing ActionListener. So one ActionListener subclass
matches one menu item.

Thank you again for your great help.
 
I

Ingo R. Homann

Hi,
<new-code>
private String[] = memo{"No Memo 1", "No Memo 2"};
</new-code>

That's it. I cannot see how to spare other lines of code. I still need:

Mapper save1Mapper = new Mapper()
{
public void menuItemAction()
{
memo[0] = theText.getText(); //replace memo1 by memo[0]
}
};

Mapper save2Mapper = new Mapper()
{
public void menuItemAction()
{
memo[1] = theText.getText(); //replace memo2 by memo[1]
}
};
Click to expand...

Try something like this (pseudocode):

Mapper[] saveMapper=new Mapper[2];
for(int i=0;i<saveMapper.length;i++) {
saveMapper=new Mapper() {
public void menuItemAction() {
memo = theText.getText();
}
}
}

OK, you do not spare *exactly* half of the code. (But another advantage
is that e.g. you could easily add a third memo without one more line of
code...)

(Note that perhaps you need a final 'i'-Variable due to (IMHO) a lack in
the langspec (*)...)

(*) Yes, I *do* know the background for this, but despite of this I
think that it is a lack in the langspec (because the compiler could
easily translate it into a working version...)
Fully agree. I realized in this situation I don't need the interface
Mapper. There is one already for me to use. That is ActionListener. I
can use several inner classes (or ananymous classes directly in the
parameter) implementing ActionListener. So one ActionListener subclass
matches one menu item.
Click to expand...

Exactly! (But I think, it was a good idea to implement it the way you
did, because it gives many insights...)

Ciao,
Ingo
 
S

Shawn

Ingo said:
Hi,
<new-code>
private String[] = memo{"No Memo 1", "No Memo 2"};
</new-code>

That's it. I cannot see how to spare other lines of code. I still need:

Mapper save1Mapper = new Mapper()
{
public void menuItemAction()
{
memo[0] = theText.getText(); //replace memo1 by memo[0]
}
};

Mapper save2Mapper = new Mapper()
{
public void menuItemAction()
{
memo[1] = theText.getText(); //replace memo2 by memo[1]
}
};
Click to expand...

Try something like this (pseudocode):

Mapper[] saveMapper=new Mapper[2];
for(int i=0;i<saveMapper.length;i++) {
saveMapper=new Mapper() {
public void menuItemAction() {
memo = theText.getText();
}
}
}

OK, you do not spare *exactly* half of the code. (But another advantage
is that e.g. you could easily add a third memo without one more line of
code...)

(Note that perhaps you need a final 'i'-Variable due to (IMHO) a lack in
the langspec (*)...)

(*) Yes, I *do* know the background for this, but despite of this I
think that it is a lack in the langspec (because the compiler could
easily translate it into a working version...)
Fully agree. I realized in this situation I don't need the interface
Mapper. There is one already for me to use. That is ActionListener. I
can use several inner classes (or ananymous classes directly in the
parameter) implementing ActionListener. So one ActionListener subclass
matches one menu item.
Click to expand...

Exactly! (But I think, it was a good idea to implement it the way you
did, because it gives many insights...)

Ciao,
Ingo
Click to expand...

Thank you very much. I greatly appreciate it. It is very helpful to me.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Could you help on this reflection technique? 13
Help with my first Java program 22
Making Java programs accessible for visually impaired users 2
Connected SQLite to my java program but information are not submitted 2
Paint Method 2
"cannot find symbol" on menuItem for ActionListener 7
Could you comment on my little program? What design pattern it is? 5
Implementing Many Stacks in the Same Program 1

Members online

Total: 61 (members: 2, guests: 59)
Robots: 399

Forum statistics

Threads
473,968
Messages
2,570,154
Members
46,702
Latest member
LukasConde

Latest Threads

Top