KeyBinding Help

[Lil_Aziz1]

I finally got the balls to read up on Key Binding. I used it to traverse through a sudoku grid using the arrow keys; however, I'm not satisfied. The program works, but I think there is a better way to use Key Bindings to do this; I don't know how or what it is. Here is my implementation of it:

public class SudokuGrid extends JPanel {


        private final JTextField[][] grid = new JTextField[9][9];
        /* Other fields are not shown */

        public SudokuGrid() {
             ....
             implementKeyBindings();
	}

	private void implementKeyBindings() {
		for (int y = 0; y < 9; y++)
			for (int x = 0; x < 9; x++) {
				grid[y][x].getInputMap().put(KeyStroke.getKeyStroke("RIGHT"), "goRight");
				grid[y][x].getInputMap().put(KeyStroke.getKeyStroke("LEFT"), "goLeft");
				grid[y][x].getInputMap().put(KeyStroke.getKeyStroke("UP"), "goUp");
				grid[y][x].getInputMap().put(KeyStroke.getKeyStroke("DOWN"), "goDown");
				
				Action direction = new SudokuTraversalPolicy("RIGHT", x, y);
				grid[y][x].getActionMap().put("goRight", direction);
				
				direction = new SudokuTraversalPolicy("LEFT", x, y);
				grid[y][x].getActionMap().put("goLeft", direction);
				
				direction = new SudokuTraversalPolicy("UP", x, y);
				grid[y][x].getActionMap().put("goUp", direction);
				
				direction = new SudokuTraversalPolicy("DOWN", x, y);
				grid[y][x].getActionMap().put("goDown", direction);
			}
	}
        /* Other methods/nested classes are not shown */

	@SuppressWarnings("serial")
	private class SudokuTraversalPolicy extends AbstractAction {

		private String arrowKey;
		private int x, y;
		
		public SudokuTraversalPolicy(String arrowKey, int x, int y) {
			this.x = x;
			this.y = y;
			this.arrowKey = arrowKey;
		}
		
		public void actionPerformed(ActionEvent e) {
			if (arrowKey.equals("RIGHT"))
				grid[y][(x == 8) ? 0 : x+1].requestFocusInWindow();
			else if (arrowKey.equals("LEFT"))
				grid[y][(x == 0) ? 8 : x-1].requestFocusInWindow();
			else if (arrowKey.equals("UP"))
				grid[(y == 0) ? 8 : y-1][x].requestFocusInWindow();
			else
				grid[(y == 8) ? 0 : y+1][x].requestFocusInWindow();
		}
	}

To differentiate between JTextFields, I was thinking of putting their position in setActionCommand; however, it turns out that getActionCommand returns the value (grid[y][x].getText()) of the JTextField, not its ActionCommand when using grid[y][x].setAction(someAbstractAction). If one uses grid[y][x].addActionListener(someActionListener), the ActionEvent returns the String set by setActionCommand.

It be very appreciated if someone could tell me a more elegant way of doing this. Thanks in advance!

[Lil_Aziz1]

I started playing with Key Binding and this is how far I got:

        private final JTextField[][] grid = new JTextField[9][9];
        /* Other fields are not shown */

        public SudokuGrid() {
             ....
                this.getInputMap(JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT).
                     put(KeyStroke.getKeyStroke("T"), "t");
		this.getActionMap().put("t", new SudokuTraversalPolicy("T");
	}
.....
	@SuppressWarnings("serial")
	private class SudokuTraversalPolicy extends AbstractAction {

		private String arrowKey;
		
		public SudokuTraversalPolicy(String arrowKey) {
			this.arrowKey = arrowKey;
		}

		public void actionPerformed(ActionEvent e) {
			if (arrowKey.equals("T")) {
				JTextField test = (JTextField) KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusOwner();
				System.out.println(test);
			}

What I've accomplished is that I got the focused component, identified that it's a JTextField, and now I need to know what its position. Should I make a new class that extends JTextField and has 2 additional fields, private int x, y; ? Or can I do something else that could make each JTextField unique enough for me to identify where it is? Thanks in advance!

EDIT: getName() & setName() !

Okay now that's better. But is there an even better solution to this?

[camickr]

I'll let you decide if this is more "elegant" or not. The main benefit of this approach is that you only define 4 Actions. The Actions are reused by each of the text fields. The Action class extends TextAction to take advantage of the getFocusedComponent() method.

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

public class SudokuPanel extends JPanel
{
	private final static int GRID_SIZE = 9;
	private final static int GRID_OFFSET = GRID_SIZE - 1;

	public SudokuPanel()
	{
		Action goRight = new SudokuTraversalPolicy("Right", 0, 1);
		Action goLeft = new SudokuTraversalPolicy("Left", 0, -1);
		Action goDown = new SudokuTraversalPolicy("Down", 1, 0);
		Action goUp = new SudokuTraversalPolicy("Up", -1, 0);

		setLayout( new GridLayout(0, GRID_SIZE) );

		for (int i = 0; i < GRID_SIZE * GRID_SIZE; i++)
		{
			JTextField textField = new JTextField(1);

			InputMap im = textField.getInputMap();
			im.put(KeyStroke.getKeyStroke("RIGHT"), "goRight");
			im.put(KeyStroke.getKeyStroke("LEFT"), "goLeft");

			ActionMap am = textField.getActionMap();
			am.put("goRight", goRight);
			am.put("goLeft", goLeft);

        	add( textField );
		}

		InputMap im = getInputMap(JPanel.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT);
		im.put(KeyStroke.getKeyStroke("DOWN"), "goDown");
		im.put(KeyStroke.getKeyStroke("UP"), "goUp");

		ActionMap am = getActionMap();
		am.put("goDown", goDown);
		am.put("goUp", goUp);
	}

	private class SudokuTraversalPolicy extends TextAction
	{
		private int rowChange;
		private int columnChange;


		public SudokuTraversalPolicy(String name, int rowChange, int columnChange)
		{
			super( name );
			this.rowChange = rowChange;
			this.columnChange = columnChange;
		}

		public void actionPerformed(ActionEvent e)
		{
			JTextComponent textField = getFocusedComponent();

			JPanel parent = (JPanel)textField.getParent();
			Component[] components = parent.getComponents();
			int location = 0;

			for (int i = 0; i < components.length; i++)
			{
				if (textField == components[i])
				{
					location = i;
					break;
				}
			}

			int row = location / GRID_SIZE;
			int column = location - (row * GRID_SIZE);

			row += rowChange;
			row = row < 0 ? GRID_OFFSET : row > GRID_OFFSET ? 0 : row;

			column += columnChange;
			column = column < 0 ? GRID_OFFSET : column > GRID_OFFSET ? 0 : column;

			location = (row * GRID_SIZE) + column;
			components[location].requestFocusInWindow();
		}
	}

	public static void main(String[] args)
	{
		JFrame frame = new JFrame();
		frame.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE );
		frame.add( new SudokuPanel() );
		frame.pack();
		frame.setLocationRelativeTo( null );
		frame.setVisible(true);
	}
}

The right/left Actions are bound to the text field because you want to replace the default text field Action for those keys. The up/down keys don't have a default Action so they can be bound to the panel.

Another benefit of using Actions with this design is that you can create Actions with other features. For example to move the focus to the first text field on the row you could do:

Action goStart = new SudokuTraversalPolicy("Start", 0, 9);
im.put(KeyStroke.getKeyStroke("control S"), "goStart");
am.put("goStart", goStart);

[Lil_Aziz1]

:O There's a TextAction class?! Couple of things though. What's the point of having a String constructor? You can't even retrieve that String. ;\ I love the idea of the rowChange and columnChange. Also, I don't get why the right and left arrow keys are in the text field's input map while the up and down are in the JPanels? What default Actions are bound to a TextField for the right and left key? Moving of the cursor? If so, what happens if you don't replace em? I'll try it out right now.

Also, I must have a 2D array to be consistent with my other classes. I have a for loop in the SudokuGrid's constructor that does that. I'll add it to that.

Another thing. Instead of using a for loop to find the location, I'm going to add the location to the JTextField's name in the SudokuGrid constructor's for loop using setName().

Going to work on this again and I will definitely implement your astute idea of row/columChange and TextAction.

EDIT: It doesn't work with the right and left when I put em in the JPanel's map. :O Cool stuff. Also, wouldn't it be best to implement the up and down to the JTextField's InputMap to stay consistent?

[camickr]

What's the point of having a String constructor?

When you add an Action to a button or menu item, this is the value that appears on the component.

You can't even retrieve that String.

The getValue(...) methed is used to retrieve the various properties of an Action.

Also, I don't get why the right and left arrow keys are in the text field's input map while the up and down are in the JPanels?

The InputMap of a Component that has focus takes precedence over the InputMap of the ancestor. So if you add the right/left Actions to the parent they will be ignored.

What default Actions are bound to a TextField for the right and left key? Moving of the cursor?

Yep.

Another thing. Instead of using a for loop to find the location, I'm going to add the location to the JTextField's name in the SudokuGrid constructor's for loop using setName().

That was the "elegant" part of the solution, so you didn't have to hack the setName() field to store a String value that needs to be parsed into row/column values :)

Also, I must have a 2D array to be consistent with my other classes.

You may need it for other

[Lil_Aziz1]

When you add an Action to a button or menu item, this is the value that appears on the component.

Oh okay. I remember reading that. It won't do that to a text field though right?

That was the "elegant" part of the solution, so you didn't have to hack the setName() field to store a String value that needs to be parsed into row/column values

Oh. I thought it was to avoid the 4 conditional statements. Isn't it more inefficient to get an array of the components and then iterating over those components to find the location?

[Lil_Aziz1]

Also what do you think about adding the up and down key bindings into the textfield's map instead of the jpanel's map? Would it be more efficient?

[camickr]

Isn't it more inefficient ...

Sure, it might add a millisecond or two to do the focus change. I doubt you will even see a blip in the CPU usage of your computer. I don't worry about micro optimization of code. I worry more about design and maintenance.

Using the setName() field is a hack. If you really want to store the row/column value then you can:

a) do as you suggested earlier which is to extend JTextField to store these values. But now you are getting into creating extra classes

b) use the putClientPropert(...) and getClientProperty(...) methods to store these values. But then behind the scenes this method will create a HashTable to store the data so now you are adding extra overhead in terms of memory.

However, I think either of the above approaches is better than a quick hack. The choice is yours.

Again, same comment with the up/down arrow key bindings. Sure they can be added to the InputMap of the text field. You might save another millisecond in search time to find the correct Action to execute. but know you 324 entries instead of 4 in the InputMap and ActionMap. I would do whatever you think is easier to understand from a design point of view.

[Lil_Aziz1]

This is my final draft I guess:

        /* Irrelevant content not shown */
        private final JTextField[][] grid = new JTextField[9][9];

	public SudokuGrid() {
                /* Irrelevant content not shown */
		Action rightArrow = new SudokuTraversalPolicy("RIGHT", 1, 0), 
				leftArrow = new SudokuTraversalPolicy("LEFT", -1, 0), 
				upArrow = new SudokuTraversalPolicy("UP", 0, -1), 
				downArrow = new SudokuTraversalPolicy("DOWN", 0, 1);

		for (<[I]Some condition is met[/I]>) {
                        /**
                         *  Each JTextField contains its position in setName() and its input/action
                         *  map contain rightArrow and leftArrow. 
                         */
		}

		InputMap im = this.getInputMap(JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT);
		ActionMap am = this.getActionMap();
		im.put(KeyStroke.getKeyStroke("UP"), "U");
		im.put(KeyStroke.getKeyStroke("DOWN"), "D");
		
		am.put("U", upArrow);
		am.put("D", downArrow);
	}
        /* Irrelevant content not shown */

	@SuppressWarnings("serial")
	private class SudokuTraversalPolicy extends TextAction {

		private int x, y;
		
		public SudokuTraversalPolicy(String arrowKey, int x, int y) {
			super(arrowKey);
			this.x = x;
			this.y = y;
		}
		public void actionPerformed(ActionEvent e) {
			JTextField test = (JTextField) this.getFocusedComponent();
			int dX = Integer.parseInt(test.getName()) / 10 + x,
				dY = Integer.parseInt(test.getName()) % 10 + y;
			grid[(dY >= 0 && dY < 9) ? dY : (y == 1) ? 0 : 8]
					[(dX >= 0 && dX < 9) ? dX : (x == 1) ? 0 : 8]
							.requestFocusInWindow();
		}
	}
}

[camickr]

grid[(dY >= 0 && dY < 9) ? dY : (y == 1) ? 0 : 8]
	[(dX >= 0 && dX < 9) ? dX : (x == 1) ? 0 : 8]
		.requestFocusInWindow();

Sorry, I just don't understand why people code like that. Why do people think they need to cram all the code into a single statement. Generally, I find if a statement needs to be split over multiple lines, then it is too complex. How do you ever debug code like this to see if the index values are correct. What is wrong with:

int row = ...
int column = ...
grid[row, column].requestFocusInWindow();

Well, thats the end of my $0.02 for this posting :)

[Lil_Aziz1]

Idk. I usually cram code if it's not too much and if I'm going to use a function only once. Honestly, those lines are easily readable. It's spaced out and broken down into several reasonable lines. This is what I would consider superfluous:

grid[(Integer.parseInt(test.getName()) % 10 + y >= 0 && Integer.parseInt(test.getName()) % 10 + y < 9) ? Integer.parseInt(test.getName()) % 10 + y : (y == 1) ? 0 : 8]
		[(Integer.parseInt(test.getName()) / 10 + x >= 0 && Integer.parseInt(test.getName()) / 10 + x < 9) ? Integer.parseInt(test.getName()) / 10 + x : (x == 1) ? 0 : 8]
				.requestFocusInWindow();

[camickr]

It's spaced out and broken down into several reasonable lines

If you need to break it down into several lines to make it readable then you've just proven my point, you are trying to cram too much into a single statement.

I honestly don't know how you can suggest the code is readable given that the indexes into the array are defined on two separate lines. You may understand the code because you wrote it, but anyone maintaining the code, or answering a question on the forum (ie. me) will need to do a double take to try and understand what you are trying to do. In a real application chances are other people will be doing the maintenance which is why readability is important.

[Lil_Aziz1]

Touche.

If it makes you feel better, I did change it:

		public void actionPerformed(java.awt.event.ActionEvent e) {
			JTextField test = (JTextField) this.getFocusedComponent();
			int dX = Integer.parseInt(test.getName()) / 10 + x,
				dY = Integer.parseInt(test.getName()) % 10 + y,
				row = (dY >= 0 && dY < 9) ? dY : (y == 1) ? 0 : 8,
				column = (dX >= 0 && dX < 9) ? dX : (x == 1) ? 0 : 8;
			grid[row][column].requestFocusInWindow();
		}