[toggle]

Hello,

In my previous assignment, I was marked down because I didn't use actionListeners correctly. The assignment included 3 buttons, each of which did something different when pressed. I placed all my code for each button inside 1 action listener class with several if statements.

For example:

class buttonPressed implements ActionListener {

		   public void actionPerformed(ActionEvent a){

			  if (a.getSource() == add){
				 checkWordAdd(input.getText());

			  }

			  if (a.getSource() == first){
				  checkLetter(toArray(al), input.getText().toLowerCase());
			  }
  
			  if (a.getSource() == many){
				  center.setText(input.getText() + " was found " + addAmount(toArray(al), input.getText().toLowerCase()) + " times");

			  }
			  
			  }
		   }
	   }

I should have made separate actionListener classes, assigned to different buttons. Like so:

class buttonAdd implements ActionListener {

		   public void actionPerformed(ActionEvent a){

			  if (a.getSource() == add){
				 checkWordAdd(input.getText());

			  }
			  
			  }
		   }
	   }

class buttonLetter implements ActionListener {

		   public void actionPerformed(ActionEvent a){

			 if (a.getSource() == first){
				  checkLetter(toArray(al), input.getText().toLowerCase());
			  }

			  }
			  
			  }
		   }
	   }


etc... etc...
	   

The problem I am now having is what to do in my current assignment. I have to use mouse buttons and depending what button is pressed, something happens. I have done exactly the same what I did in the previous example, and am concerned I will be marked down again. I just don't know how to split these up, because there is only 1 panel I can add the mouseHandler to. (In the previous assignment, there was 3 buttons each of which could have their own buttonHandler).

Here is what I have got:

 public void mouseClicked(MouseEvent e){

	
	   if (e.getButton() == 1){							 
		  // Code to make something happen
		  
	   }

	  if (e.getButton() == 3){
			// Code to make something happen

	  }
	 

   if (e.getButton() == 2){
		  // Code to make something happen

	 }

}

The code inside these seperate if statements is very long and I didn't want to bore you with the details. The bit I am confused with is how can I split these into seperate mouse classes. They all point to the same panel. It just seems like I'll be repeating code over and over again.

I lost about 10% last time because of this mistake and I don't want to make it again.

[EdwinNameless]

Generally speaking, I prefer to have one listener per component. Stuffing all the actions within the same listener usually leads to a long and painful litany of if ... else which is hard to maintain.

To avoid getting swamped under the number of classes, I'd use anonymous inner classes.

So say I want to add behaviour to your buttons, I'd have:

JButton add  = new JButton();
JButton first = new JButton();

// ...

add.addActionListener(new ActionListener() {
  public void actionPerformed(ActionEvent e) {
	 checkWordAdd(input.getText());
  }
});

first.addActionListener(new ActionListener() {
  public void actionPerformed(ActionEvent e) {
	 checkLetter(toArray(al), input.getText().toLowerCase());
  }
});

// etc.

The idea is the same for MouseListener. You would have something like:

component.addMouseListener(new MouseListener() {
  // implement all the MouseListener.
});

However, MouseListener has many more methods, so it's a bit of a pain, because you have to implement them all. Fortunately, MouseAdapter is an abstract class that already defines these methods for you. All you have to do is implement the one you want:

component.addMouseListener(new MouseAdapter() {
  public void mouseClicked(MouseEvent e) {
	System.out.println("About to do cool stuff...");
	//  Cool stuff
  }
});

Hope this helps a bit...

[Dogstopper]

Here are some sun tutorials to reinforce what he said:

Adapter class: http://java.sun.com/...l#eventAdapters

Inner class, like you were trying to implement: http://java.sun.com/...nerclasses.html

And personally, I have found that anonymous inner classes have been immensly useful when tracking down bugs because you have a separate listener for each element.

[toggle]

That's brilliant, thank you

[toggle]

Ok, I'm getting confused here.

I have done what you suggested and I end up with something like this in the main class:

  c.addMouseListener(new MouseAdapter() {
			public void mouseClicked(MouseEvent e) {
	  if (print){
	   if (e.getButton() == 1){						 
		  

		 // 15 lines of confusing code

	 }
  }
});
c.addMouseListener(new MouseAdapter() {
			public void mouseClicked(MouseEvent e) {
	  if (print){
	   if (e.getButton() == 2){						 
		  

		 // 15 lines of confusing code

	 }
  }
});
c.addMouseListener(new MouseAdapter() {
			public void mouseClicked(MouseEvent e) {
	  if (print){
	   if (e.getButton() == 3){						 
		  

		 // 15 lines of confusing code

	 }
  }
});

It just looks really messy compared to:

c.addMouseListener(new MouseHandler());

// and inside class MouseHandler:

 public void mouseClicked(MouseEvent e){

																
	 if (comp.print){
	   if (e.getButton() == 1){							 
		   

// 15 lines of code


	 }

															  
	 if (comp.print){
	   if (e.getButton() == 2){							 
		   

// 15 lines of code


	 }
															  
	 if (comp.print){
	   if (e.getButton() == 3){							 
		   

// 15 lines of code


	 }
 
}

My main class already is really long, adding in all the junk above makes it look pretty confusing. Even with adding seperate MouseAdapters to the same panel (for different clicks) I still need to put and if statement

if (e.getSource == 1)

to determine what button is being pressed.

[EdwinNameless]

You don't need the if (e.getButton() == 1){s any more, as the MouseListener is attached to the button, so the source is necessarily that button.

Also, the cool thing about inner classes is that they "see" methods of the enclosing class. So if the buttons do something similar, you can extract a method, and calling like this:

public void doStuff() {
// ...
add.setText("Add");
add.addActionListener(new ActionListener() {
  public void actionPerformed(ActionEvent e) {
	fun();
  }
});
// ...
}

public void fun() {
  System.out.println("fun!");
}

This should clean up a bit if you have common behaviour in the listeners.

This post has been edited by EdwinNameless: 01 December 2009 - 05:54 AM

[toggle]

The problem is, It's not a button being pushed its a mouse button.

If the left mouse button is clicked (anywhere on the panel) it does something, if the right mouse button is clicked (anywhere on the panel) something else happens.

I think I still need an if statement to determine which mouse button is pressed. I apologies if I'm being dumb and missing your point here.

[EdwinNameless]

toggle, on 1 Dec, 2009 - 04:56 AM, said:

The problem is, It's not a button being pushed its a mouse button.

If the left mouse button is clicked (anywhere on the panel) it does something, if the right mouse button is clicked (anywhere on the panel) something else happens.

I think I still need an if statement to determine which mouse button is pressed. I apologies if I'm being dumb and missing your point here.

Oh I see what you're saying, I was still in the ActionListener world. Then you would need these ifs indeed, but you can still extract the common parts in external methods.

On the other hand, if all the components do the same thing, or if the logic in the listener is to be reused somewhere else, then you'd be better off having indeed another class, extending MouseAdapter, or implementing MouseListener depending on your needs.

This post has been edited by EdwinNameless: 01 December 2009 - 06:07 AM

[toggle]

Righto.

So from a Object Orientated Programming point of view, which of these is more efficient.

Option 1:

//This is the main class: 

  c.addMouseListener(new MouseAdapter() {
			public void mouseClicked(MouseEvent e) {
				if (e.getButton() == 1){
				m.left();
				}
		 }});

		c.addMouseListener(new MouseAdapter() {
			public void mouseClicked(MouseEvent e) {
				if (e.getButton() == 2){
				m.up();
				}
		 }});

  c.addMouseListener(new MouseAdapter() {
			public void mouseClicked(MouseEvent e) {
				if (e.getButton() == 3){
				m.right();
				}
		 }});

//Inside the method class:

left(){
//lots of code
}

up(){
//lots of code
}

right(){
//lots of code
}

Option 2

//Inside the main class:

c.addMouseListener(new MouseHandler())


//Inside the MouseHandler class:


public void mouseClicked(MouseEvent e){

	   if (e.getButton() == 1){							 
		   
// Lots of code

	 }

	  if (e.getButton() == 21){							 
		   
// Lots of code

	 }

  if (e.getButton() == 3){							 
		   
// Lots of code

	 }

}

By what you are suggesting, the first option is correct. I assume this is because it is easier to see at a glance what methods are being run each time a certain button is being clicked?
If thats the case, could you do this instead (or does it devalue the whole purpose why you split it up in the first place):

c.addMouseListener(new MouseAdapter() {
			public void mouseClicked(MouseEvent e) {
				if (e.getButton() == 1){
				m.left();
				}
				if (e.getButton() == 2){
				m.up();
				}
				if (e.getButton() == 3){
				m.right();
				}
	   
		 }});

This way, you are reducing three almost identical segments of code into one. Which is why I "assume" it would be better, because you are reducing and reusing existing code.

Sorry for all the questions. I'm just trying to make sure I understand this 100%

This post has been edited by toggle: 01 December 2009 - 06:23 AM

[EdwinNameless]

The third option would be preferable to the first one, because you are attaching 3 different mouse listeners to the _same_ component (in my examples, I had different components). I'd rather have all the behaviour for the same component in the same listener. Option 2 is a refinement of option 3 in that it gives you the opportunity to extract some common methods.

Again, this depends on how often this listener will crop up in the code. If the listener is a one-off, that applies to this component only, a anonymous inner class is good; I find this is the most common case. If this is something that is called several times, you'll want to have a standalone class so that you can reuse this listener in different places.

I hope I didn't confuse you too much...

[toggle]

Brilliant, thank you.

I will stick to the third option and put the methods in a different class. I am doing this mostly to make it look less complicated and bloated. There is a lot of code in those methods.

Thank you

[Dogstopper]

Another option not mentioned 2 posts ago was the option of having another class implement ALL of the event code. By literally defining 1 class inside of the other (called nested classes), you separate the code appropriately, but still have access to the outer class's private variables. I am using this option now as I subclass the Swing library, because I use loads of methods to help that "listener"... Here is the shortened/cut version of that class to demonstrate my point.

public class TransparentFrame extends JFrame {

	public TransparentFrame(String title) {

	   ...
		TFrameHandler tfh = new TFrameHandler();
		this.addComponentListener(tfh);
		this.addWindowFocusListener(tfh);
		this.addWindowListener(tfh);
	}

... Lots of code!

	 // NOTE all methods in this class are really implemented, I merely shortened it.
	class TFrameHandler implements
			ComponentListener, WindowFocusListener, WindowListener {
		
		public TFrameHandler() { }

		private Image getElement() { }

		// Methods that handle the invisibility
		private void refresh() { }

		// repaints the panel
		private void updatePanel() { }

		public void componentResized(ComponentEvent e) { }
		public void componentMoved(ComponentEvent e) { }
		public void componentShown(ComponentEvent e) { }
		public void componentHidden(ComponentEvent e) { }
		public void windowGainedFocus(WindowEvent e) { }
		public void windowLostFocus(WindowEvent e) { }
		public void windowOpened(WindowEvent e) { }
		public void windowClosing(WindowEvent e) { }
		public void windowClosed(WindowEvent e) { }
		public void windowIconified(WindowEvent e) { }
		public void windowDeiconified(WindowEvent e) { }
		public void windowActivated(WindowEvent e) { }
		public void windowDeactivated(WindowEvent e) { }
	}
}

Notice how all of my event handling code is in its own class with its own separate handler methods? This allows it to be clean, while still being able to access variables in the outer class.

Hope you learned something!