• Post Reply Bookmark Topic Watch Topic
  • New Topic

actionlistener doesn't work with button click, but doclick() does  RSS feed

 
Kevin Kahl
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have buttons created on a frame and then I register the listener from a controller in a controller-view relationship.

When I click the button on the face, the action never executes for some reason. I thought maybe there was a problem registering the listener but I can call the buttons doclick() method and it executes as it should. Perhaps I'm overlooking something really obvious here but I can't see it.



and then I add the listener



again, when i click the button nothing happens. but if i add the following code



the actionPerformed invokes in theController as I intended.

You can see the entire project on GitHub so you can see the full context. The controller class contains the listener and the view class contains the buttons.
 
Paul Clapham
Sheriff
Posts: 22185
38
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I like your original way of assigning the listener to the button -- the commented-out line 2 in your first code fragment. Does that not work? I would get the normal way of adding listeners to buttons working correctly before converting it to obscure code like what you posted there.
 
Piet Souris
Rancher
Posts: 1783
55
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi Kevin,

the error is a subtle one, but it is one that I see from time to time at this site.
The error boils down to the following template:

Have a look at your main-method:

So you have the line 'View theView = new View()';
Okay. What I want you to do now is to give the method
'View.CalcView()' a thorough inspection, taking my opening text
into consideration.

The fix is very easy, though, taking no more than just a few
key strokes. Keep us informed!

Greetz,
Piet
 
Kevin Kahl
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Oh yes yes yes! I had been suspicious of that method. It's because I'm creating a second instance of view isn't it? Remove the instantiation and set and voila!
 
Kevin Kahl
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:I like your original way of assigning the listener to the button -- the commented-out line 2 in your first code fragment. Does that not work? I would get the normal way of adding listeners to buttons working correctly before converting it to obscure code like what you posted there.


Paul, my launcher class instantiates the view, model, and then passes those two into the controller. If I create (as you see it) during the class instantiation, it creates a null pointer exception because has not been created yet. I learned a valuable lesson during this project in regards to creating objects that depend on objects that do not exist at the time of creation. It occurred to me to explicitly set the controller in the method I reference above (btnOne.addActionListener( controller ), btnTwo.AddActionListener( controller ), but I felt that I shouldn't have it so concrete. Perhaps this is not right, however.
 
Paul Clapham
Sheriff
Posts: 22185
38
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Kevin Kahl wrote:If I create (as you see it) during the class instantiation, it creates a null pointer exception because has not been created yet. I learned a valuable lesson during this project in regards to creating objects that depend on objects that do not exist at the time of creation.


Fair enough, that's a good point. I probably would have reacted to that problem by creating the controller first, though.
 
Kevin Kahl
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, I pass the model and view into the controller's constructor so If created the controller first, i'd pass a null model and view. I suppose there is a way to do this without using the constructor, such as create all three classes and then tell each class about the other, but the initial template I began with started this way.
 
Piet Souris
Rancher
Posts: 1783
55
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Kevin Kahl wrote:Oh yes yes yes! I had been suspicious of that method. It's because I'm creating a second instance of view isn't it? Remove the instantiation and set and voila!

hi Kevin,

this was exactly what I meant. Well done for spotting it.

Since you supplied a GitHub-address, Paul can have a look at your complete code, just
like I did. Then he can see what it is that you are trying to achieve (and how you try it!).

There are improvements to be made. First of all, I would not have used a null layout
for the view, which I would have let extend a JPanel, using a suitable LayoutManager.

And I would have liked to see a dedicated ActionListener in stead of a KeyListener that
implements an ActionListener.

But these are nice to try out later. Meanwhile, have a look at this tutorial:

http://docs.oracle.com/javase/tutorial/uiswing/misc/keybinding.html

Give this a read, and let us know what you think of it. Do you think you could
apply it somehow?

Greetz,
Piet
 
Kevin Kahl
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Piet Souris wrote:
Kevin Kahl wrote:Oh yes yes yes! I had been suspicious of that method. It's because I'm creating a second instance of view isn't it? Remove the instantiation and set and voila!

hi Kevin,

this was exactly what I meant. Well done for spotting it.

Since you supplied a GitHub-address, Paul can have a look at your complete code, just
like I did. Then he can see what it is that you are trying to achieve (and how you try it!).

There are improvements to be made. First of all, I would not have used a null layout
for the view, which I would have let extend a JPanel, using a suitable LayoutManager.

And I would have liked to see a dedicated ActionListener in stead of a KeyListener that
implements an ActionListener.

But these are nice to try out later. Meanwhile, have a look at this tutorial:

http://docs.oracle.com/javase/tutorial/uiswing/misc/keybinding.html

Give this a read, and let us know what you think of it. Do you think you could
apply it somehow?

Greetz,
Piet


Hey Piet, thanks for your help with this.

Ok, per advice received had planned on abandoning the keylistener aspect and move to key bindings but they are on the back burner for now. This is simply because I have had not had the time to study them and I better actually get the damn thing to perform some calculations ha.

As far as the view goes, I really have no idea what I'm doing. This started out as an exercise to build a calculator and I used an eclipse plugin to create the view. Then, I started looking at design patterns and decided to try to make this a model-view-controller. I had to chop the code up a bit in the view so it was the default launching class. I could redesign it. How do you mean a null layout for view? I thought I chose absolute. Or maybe I didn't. I'm not sure how the panel plays into this as I have really just text based operations experience. I have a lot to learn.
 
Kevin Kahl
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So I have been learning about implementing key bindings in my basic because I want to enable users to use the number pad as well as press button on the frame. The same advice I always get is to use key bindings as mentioned here and elsewhere.

So I started in with this and realized that I could do this a number of ways:

1. Write one Abstract Action that takes all input, checks what it is and sends it on to the appropriate method. So I'll have around 16 key bindings all point to one Input Abstract Action - This way seems counter intuitive to using a key binding in the first place? Why couldn't I use a key listener or something to just capture what is pressed.

2. Or create a separate Abstract Action for each key binding. This seems appropriate for the use of key bindings, but it seems like a lot of work for basic functionality. 16 Key bindings, 16 abstract methods.

3. ??
So what is a best practice here? I've implemented the abstract classes in my controller and am registering the bindings via a method call from the main program (after the classes are created).
 
Paul Clapham
Sheriff
Posts: 22185
38
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Number 1 isn't a best practice. You'll see that sort of thing in really old Swing tutorials where you have one ActionListener which contains a monstrous if-else-if-else structure. And neither is #2, really. What is best practice is to have one AbstractAction for each, um, each action which the keys are supposed to trigger.

So if you have 16 keys which are meant to trigger 16 different actions, you need 16 AbstractAction objects. And if all of the keys are supposed to trigger the same action, you only need one AbstractAction. Consider a calculator, for example. The + and - keys require different actions, so you'd have different AbstractActions to handle them. But the digit keys all require the same action, so you only need one AbstractAction to handle them.
 
Kevin Kahl
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:Number 1 isn't a best practice. You'll see that sort of thing in really old Swing tutorials where you have one ActionListener which contains a monstrous if-else-if-else structure. And neither is #2, really. What is best practice is to have one AbstractAction for each, um, each action which the keys are supposed to trigger.

So if you have 16 keys which are meant to trigger 16 different actions, you need 16 AbstractAction objects. And if all of the keys are supposed to trigger the same action, you only need one AbstractAction. Consider a calculator, for example. The + and - keys require different actions, so you'd have different AbstractActions to handle them. But the digit keys all require the same action, so you only need one AbstractAction to handle them.


Ok I get it. So really I'd just need, a set for operators, a set for numbers, and one for the like clear/clear all. The idea being grouping the functions then right?
 
Paul Clapham
Sheriff
Posts: 22185
38
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If + and - do the same thing, you could use the same action for both. But if that action is going to start, "If + then ... else if - then ...", you should use one action for each of them.
 
Campbell Ritchie
Sheriff
Posts: 53779
128
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote: . . . one ActionListener which contains a monstrous if-else-if-else structure. . . .
I agree that is dreadful, but why do you think you only saw that in old tutorials?
 
Kevin Kahl
Greenhorn
Posts: 23
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Just like to let everyone involved with this that I've finished the main calculator. There are some limitations and minor bugs and a few todos, but the basic is there. This started out as a lesson in MVC. I'd like to hear some critiques. Also, thanks to everyone who helped me with this.

The repo for this is here:

http://github.com/KiloJKilo/Calculator/tree/new_controller
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!