Java help - KeyListener

I’m working on a personal project to improve my Java skills. At the moment I’m trying to figure out how to implement keyboard input, but the examples I’ve seen so far haven’t been much use. I’ve made a few attempts but nothing has worked so far. With that in mind, I created a very small program to use as a stepping stone:


package testing;

import java.awt.*;
import javax.swing.*;

public class Testing extends JFrame
{
    JButton movingButton;
    
    Testing()
    {
        setLayout(null);
        setSize(500,500);
        
        movingButton = new JButton("This Button Moves");
        
        add(movingButton);
        movingButton.setBounds(150,150,200,100);
    }
    
    public static void main(String[] args)
    {
        Testing gui = new Testing();
        gui.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        gui.setVisible(true);
        gui.setTitle("Test Window");
    }
}

Basically, I’m trying to figure out what I need to change so that if the GUI window is selected, pressing the Left key changes movingButton.setLocation to 100,150.

I’d like to note that Swing… isn’t very good. I’d recommend finding a different library (I hear JavaFX is good).

It has been a super long time since I’ve done Java, and doubly long since I’ve done Swing, but I believe what you want is something along the lines of:



private class MyKeyListener implements KeyListener {
  private JButton button;
  
  public MyKeyListener(JButton button) {
      this.button = button;
  }

  public void keyPressed(KeyEvent e) {
     if( e.getKeyCode() == KeyEvent.VK_LEFT ) {
        button.setLocation(100,500);
     }
  }

  // Does nothing
  public void keyReleased(KeyEvent e) {

  }

  // Does nothing
  public void keyTyped(KeyEvent e) {

  }
}


Put this INSIDE the Testing class. It’s known as a “private inner class” and allows you to define a listener visible only to the class using it. (You can also use something called an “anonymous class” where you define it in the constructor of the Testing class, but IMO that’s ugly).

Then in your constructor, do this:



Testing() {
   //... stuff you already wrote

   // Give keyListener a reference to your button
   keyListener = MyKeyListener(movingButton);

   // Add the listener to the JFrame.
   add(keyListener);
}


To come up with this, I used the documentation, notably Key Listener, and KeyEvent.

If any of that is unclear or you don’t understand why I did certain things, let me know and I’d be happy to explain it.

A few problems to work through…

First and foremost, your frame isn’t getting the event because the button has focus. As long as the button has focus (as the only component in the frame), the key listener you add to the frame will not receive the event.
There are ways around this, and that’s left as an exercise for the reader
(ETA: I guess the frame isn’t getting the event because the event handler was never added :slight_smile: )

I will differ with my esteemed colleague Jragon and recommend using the less pretty anonymous inner classes. The reason for this is that in serious Swing programming you need to add tons of these listeners all over the place and you really don’t want to have to formally define each one.
The nice tools (Eclipse and Netbeans) allow you to type in the inner class stuff and will automatically provide the methods.
For example I say:


KeyListener listener = new KeyListener(){};

Then Eclipse or Netbeans will whine and say I haven’t implemented required methods (after possibly importing KeyListener for me), and I say “make it so” and the tool adds the other stuff.

Java 8 will make much of this nicer with closures, or so I have heard. And it will be very familiar to those who work with anonymous inner classes.

Finally, your Swing stuff must all be run from the event thread. It is very common for developers to forget this in the main().

You should do something like this:


Runnable runnable = new Runnable(){
  public void run(){
    MyFrame frame = new MyFrame();
    frame.setVisible(true);
  }
}
SwingUtilities.invokeLater(runnable);

(and thus another anonymous inner class motif shows up)

The reason I prefer not using anonymous classes is because whenever I use them I lose track of the constructor. It becomes this giant monster function. I’ve seen a lot of Swing programs with bugs that stem from accidentally adding something outside of the constructor because the thing is so huge.

I remember one program in high school, it was relatively simple: it had a couple text boxes and some sliders. It converted temperatures. Entering a number in one box or moving the corresponding slider would convert it to the opposite temperature and set everything to the correct value. It would also change the screen background color depending on how hot/cold it was. Even with good design, the constructor gets to bonkers length with anonymous classes.

This is a matter of finesse, though. In real life, for THIS specific case, I’d use an anonymous class. I start splitting things into different classes (and occasionally different files) once things start becoming monstrous. I do this with closures too in languages that allow it, if I end up with too many closures/anonymous functions in one function, I’ll start spinning them off into generator functions just to maintain sanity.

I recommended not using an anonymous class not because I’m inherently against them, but because when you’re not good at gauging when a constructor is “too massive” it ends up cleaner.

I succeeded in creating a program that compiles and does what I was trying to do:


package testing;

import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

public class Testing extends JFrame
{
    private JFrame mainFrame;
    private JButton movingButton;
    
    public Testing()
    {
        mainFrame = new JFrame("Test Window");
        movingButton = new JButton("This button moves!");
        movingButton.setBounds(150,150,200,100);
        
        mainFrame.setLayout(null);
        mainFrame.setSize(500,500);
        
        Container c = mainFrame.getContentPane();
        c.add(movingButton);
        
        movingButton.addKeyListener(new KeyAdapter()
        {
            @Override
            public void keyPressed(KeyEvent e)
            {
                if(e.getKeyCode() == KeyEvent.VK_LEFT)
                {
                    movingButton.setLocation(movingButton.getX()-20, movingButton.getY());
                }
                else if(e.getKeyCode() == KeyEvent.VK_RIGHT)
                {
                    movingButton.setLocation(movingButton.getX()+20, movingButton.getY());
                }
                else if(e.getKeyCode() == KeyEvent.VK_UP)
                {
                    movingButton.setLocation(movingButton.getX(), movingButton.getY()-20);
                }
                else if(e.getKeyCode() == KeyEvent.VK_DOWN)
                {
                    movingButton.setLocation(movingButton.getX(), movingButton.getY()+20);
                }
            }
        });
        
        mainFrame.setVisible(true);
    }
    
    public static void main(String[] args)
    {
        Testing gui = new Testing();
        
        gui.mainFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }
}

Thank you both for your assistance. I find it much easier to work from a program that works that can be modified or optimised than trying to start from scratch or a tutorial that does not compile.

Glad you got it working. Watch out for systems that handle key events differently: sometimes key up is the one to use, other times key pressed is the one.

[soapbox]
It is important enough to repeat: Please be careful to never touch Swing objects from outside the GUI thread (as you do in your main). If multithreading is not your gig, at least use that code snippet I provided to ensure that your “new frame” code is executing within the GUI thread.

The reason for this is that Swing classes are not thread safe. The more compelling reason for this is that weird things begin to happen when you access Swing objects from multiple threads, and this is a very difficult bug to trace since it manifests itself in random quirky behavior. It won’t affect you today, but this is a good habit to establish now.

Two of the most common sources of accidental multithreading in Swing are initialization in the main (your example) and firing events from other threads (e.g. a slow running process firing a TableEvent to tell a JTable that data is ready).
[/soapbox]

I never knew about that. I wonder if this is another part of why people say Swing has terrible design. I work with a lot of systems that are thread sensitive. Notably OpenGL and OpenAL. However, none of those have a thread secretly running in the background that you have to run things on. You specify the thread you want to run on implicitly by creating the context on that thread (some newer wrapper APIs allow you to do makeContextCurrent calls to swap the thread, but the point stands).

I’ll admit, my internal Rendering system works likes this, but that’s due to a peculiar constraint on threading in the language I’m using, and towards the user everything is either thread safe or explicitly marked otherwise. (The GUI abstraction passes things in a thread-safe manner to the renderer, which is the only component that has a few non-thread-safe methods).

Noted, and changed. Is the lack of thread safety you mention only a vulnerability to race conditions?

More or less.

I have never seen anything worse happen than strangeness where a component lays itself out prematurely or labels appear funny, but … it’s easy enough to avoid these things by religiously running every GUI thing in the event thread.

Normally that’s not hard to do at all since almost all GUI code written by developers is event driven: you write code that is called by event handlers, which will by always be called by the event thread. Unless you are launching your own threads, the only place you need to worry about this rule is the main().

Even a very trivial change might cause weird race conditions. The problem is that every time you call “set” on a property in a Swing component, there is a cascade of events, and unless the component author was a belt-and-suspenders kind of person the event will be fired from within whatever thread set the property.

Imagine you call “setText” on a text field in your GUI, just to set things up.
That will cause DocumentEvent to be fired, and possibly others (e.g. “PropertyChangedEvent”).
Containers higher in the hierarchy might have registered themselves as event listeners for property change events, and they might respond to the text field change by interrogating the field (e.g. “what is your preferred size, now that your text changed”) and they might call setters on other things.

My point is that even something as innocuous as calling a setter can trigger a flood of calls that you never were aware were going to happen, and those would all happen in the incorrect thread.

One last update:

The proper program was running into some problems with race conditions if you tried to move around too quickly. Your suggestion of using invokeLater successfully got rid of that problem.