HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Implementation of KeyEventDispatcher in Java

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationkeyeventdispatcherjava

Problem

This is my first test at making a key event dispatcher. Some of the code is unnecessary if one is interested in separating lower and upper cases, however I want to treat them as the same. Hence my wonderfully named method keyToLowerCaseIfUpperCase(char c). I also want to lock the key if it has been pressed untill it is released by the user. To run an event twice the user first needs to release the key.

First of it checks if the key is in [A...Z], if so it returns the corresponding lower key. If not it will keep the key intact. Then I check whetever a key was pressed or not pressed, if it was pressed it will check if that specific key actually can be pressed (using my lock). If it was a key release it will simply release the lock of that key.

import java.awt.KeyEventDispatcher;
import java.awt.event.KeyEvent;

import java.util.HashMap;

public class KeyBindManager implements KeyEventDispatcher
{
    private HashMap keyPresses = new HashMap<>();

    @Override
    public boolean dispatchKeyEvent(KeyEvent e) {
        String pressed = keyToLowerCaseIfUpperCase(e.getKeyChar())+"";

        if (e.getID() == KeyEvent.KEY_PRESSED) {

            if (keyPresses.get(pressed) != null && keyPresses.get(pressed).booleanValue()) {
                return false;
            }
            keyPresses.put(pressed, new Boolean(true));

            switch (keyToLowerCaseIfUpperCase(e.getKeyChar())) {
                case 'a': System.out.println("D"); break; // these are just two test cases
                case 'b': System.out.println("E"); break;
                default: break;
            }

        } else if (e.getID() == KeyEvent.KEY_RELEASED){
            keyPresses.put(pressed, new Boolean(false));
        }
        return false;
    }

    private char keyToLowerCaseIfUpperCase (char c) {
        if (c >= 'A' && c <= 'Z') {
            c = Character.toLowerCase(c);
        }

        return c;
    }
}


Please tell me this can be done in a more clean approach than what

Solution

First...

I'd, personally, rely on the virtual key code over the key character. It's possible that the key character may not have been interpreted until the KEY_TYPED event (or at least I remember some issue where the char could be interpreted at certain stages)

private HashMap keyPresses = new HashMap<>();

@Override
public boolean dispatchKeyEvent(KeyEvent e) {
    int keyCode = e.getKeyCode();

    if (e.getID() == KeyEvent.KEY_PRESSED) {

        if (keyPresses.contains(keyCode) && keyPresses.get(keyCode).booleanValue()) {
            return false;
        }
        keyPresses.put(keyCode, new Boolean(true));


This also means you don't need to convert the key to lower case, as the virtual key doesn't carry the modifier information (the key character is the key code + the modifiers)

There is a problem with this, KeyEvent.VK_1 can represent both 1 and !, so you might need to consider using KeyStroke instead...
Second...

It's a nitpick on my behalf, but I like a single entry and single exit point in the code...

@Override
public boolean dispatchKeyEvent(KeyEvent e) {
    int keyCode = e.getKeyCode();

    if (e.getID() == KeyEvent.KEY_PRESSED) {

        if (!keyPresses.contains(keyCode) || !keyPresses.get(keyCode).booleanValue()) {
            keyPresses.put(keyCode, new Boolean(true));


It generally makes the code easier to read and the logic to follow as return statements can "hide" in code and it's easy to miss them (IMHO)
Third

Rather than something like this...

switch (keyToLowerCaseIfUpperCase(e.getKeyChar())) {
    case 'a': System.out.println("D"); break; // these are just two test cases
    case 'b': System.out.println("E"); break;
    default: break;
}


I would consider using another Map, to map between the virtual key and the value or "callback" you want to call. It will be easier to add new values and reuse the code

I'd also consider using a callback mechanism, which would notify of the KEY_PRESSED and KEY_RELEASED events, so the dispatcher didn't do the conversion itself, but simply managed the notification processes, further separating the implementation logic and allow the dispatcher to be more reusable.

You could achieve the same thing through abstraction, but I think some kind of Observer Pattern would be more pratical (IMHO)
Fourth

Unless you really need to know if a key is not been pressed, I would remove the keyCode from the Map on the KEY_RELEASED event

} else if (e.getID() == KeyEvent.KEY_RELEASED){
    keyPresses.remove(keyCode);
}


As it would simpler to just use something like keyPresses.contains(keyCode) instead of then actually needing to check the value associated with, but this comes down to needs.

You could replace the Map with a Set instead which would be simpler, either the keyCode exists or it doesn't...

Code Snippets

private HashMap<Integer, Boolean> keyPresses = new HashMap<>();

@Override
public boolean dispatchKeyEvent(KeyEvent e) {
    int keyCode = e.getKeyCode();

    if (e.getID() == KeyEvent.KEY_PRESSED) {

        if (keyPresses.contains(keyCode) && keyPresses.get(keyCode).booleanValue()) {
            return false;
        }
        keyPresses.put(keyCode, new Boolean(true));
@Override
public boolean dispatchKeyEvent(KeyEvent e) {
    int keyCode = e.getKeyCode();

    if (e.getID() == KeyEvent.KEY_PRESSED) {

        if (!keyPresses.contains(keyCode) || !keyPresses.get(keyCode).booleanValue()) {
            keyPresses.put(keyCode, new Boolean(true));
switch (keyToLowerCaseIfUpperCase(e.getKeyChar())) {
    case 'a': System.out.println("D"); break; // these are just two test cases
    case 'b': System.out.println("E"); break;
    default: break;
}
} else if (e.getID() == KeyEvent.KEY_RELEASED){
    keyPresses.remove(keyCode);
}

Context

StackExchange Code Review Q#80630, answer score: 6

Revisions (0)

No revisions yet.