I made simple input class that gets input from the keyboard and mouse (code might be copied). I just want to get feedback about the code.
package com.memequickie.engine;
import java.awt.Canvas;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.event.MouseMotionListener;
import java.awt.event.MouseWheelEvent;
import java.awt.event.MouseWheelListener;
public class Input implements KeyListener, MouseListener,
MouseMotionListener, MouseWheelListener {
private static final short NUM_KEYS = 256;
private static final short NUM_BUTTONS = 5;
private boolean[] keys = new boolean[NUM_KEYS];
private boolean[] keysLast = new boolean[NUM_KEYS];
private boolean[] buttons = new boolean[NUM_BUTTONS];
private boolean[] buttonsLast = new boolean[NUM_BUTTONS];
private int mouseX, mouseY;
private int scroll;
private int scale = 1;
private boolean isCtrlPressed = false;
private boolean isShiftPressed = false;
private boolean isAltPressed = false;
private boolean isEscapePressed = false;
private boolean isWheelClicked = false;
private boolean isRightClicked = false;
private boolean isLeftClicked = false;
public Input(int scale) {
mouseX = 0;
mouseY = 0;
scroll = 0;
this.scale = scale;
if(scale < 0) scale = 1;
}
public void addListeners(Canvas c) {
c.addKeyListener(this);
c.addMouseMotionListener(this);
c.addMouseListener(this);
c.addMouseWheelListener(this);
}
//Update in each game loop
public void update() {
scroll = 0;
System.arraycopy(keys, 0, keysLast, 0, NUM_KEYS);
System.arraycopy(buttons, 0, buttonsLast, 0, NUM_BUTTONS);
}
public boolean isKey(int keycode) {
return keys[keycode];
}
public boolean isKeyPressed(int keycode) {
return keys[keycode] && !keysLast[keycode];
}
public boolean isKeyReleased(int keycode) {
return !keys[keycode] && keysLast[keycode];
}
public boolean isButton(int button) {
return buttons[button];
}
public boolean isButtonPressed(int button) {
return buttons[button] && !buttonsLast[button];
}
public boolean isButtonReleased(int button) {
return !buttons[button] && buttonsLast[button];
}
public void mouseWheelMoved(MouseWheelEvent e) {
scroll = e.getWheelRotation();
}
public void mouseDragged(MouseEvent e) {
mouseX = e.getX() / scale;
mouseY = e.getY() / scale;
}
public void mouseMoved(MouseEvent e) {
mouseX = e.getX() / scale;
mouseY = e.getY() / scale;
}
public void mouseClicked(MouseEvent e) {}
public void mouseEntered(MouseEvent e) {}
public void mouseExited(MouseEvent e) {}
public void mousePressed(MouseEvent e) {
buttons[e.getButton()] = true;
if(e.getButton() == MouseEvent.BUTTON1) isLeftClicked = true;
if(e.getButton() == MouseEvent.BUTTON2) isWheelClicked = true;
if(e.getButton() == MouseEvent.BUTTON3) isRightClicked = true;
}
public void mouseReleased(MouseEvent e) {
buttons[e.getButton()] = false;
if(e.getButton() == MouseEvent.BUTTON1) isLeftClicked = false;
if(e.getButton() == MouseEvent.BUTTON2) isWheelClicked = false;
if(e.getButton() == MouseEvent.BUTTON3) isRightClicked = false;
}
public void keyPressed(KeyEvent e) {
keys[e.getKeyCode()] = true;
if(e.getKeyCode() == KeyEvent.VK_SHIFT) isShiftPressed = true;
if(e.getKeyCode() == KeyEvent.VK_CONTROL) isCtrlPressed = true;
if(e.getKeyCode() == KeyEvent.VK_ALT) isAltPressed = true;
if(e.getKeyCode() == KeyEvent.VK_ESCAPE) isEscapePressed = true;
}
public void keyReleased(KeyEvent e) {
keys[e.getKeyCode()] = false;
if(e.getKeyCode() == KeyEvent.VK_SHIFT) isShiftPressed = false;
if(e.getKeyCode() == KeyEvent.VK_CONTROL) isCtrlPressed = false;
if(e.getKeyCode() == KeyEvent.VK_ALT) isAltPressed = false;
if(e.getKeyCode() == KeyEvent.VK_ESCAPE) isEscapePressed = false;
}
public void keyTyped(KeyEvent e) {}
public int getMouseX() {
return mouseX;
}
public int getMouseY() {
return mouseY;
}
public int getScroll() {
return scroll;
}
public boolean isCtrlPressed() {
return isCtrlPressed;
}
public boolean isShiftPressed() {
return isShiftPressed;
}
public boolean isAltPressed() {
return isAltPressed;
}
public boolean isEscapePressed() {
return isEscapePressed;
}
public boolean isWheelClicked() {
return isWheelClicked;
}
public boolean isRightClicked() {
return isRightClicked;
}
public boolean isLeftClicked() {
return isLeftClicked;
}
}
-
1\$\begingroup\$ Sorry for just quick comments: not sure I like this. It appears to be "doing too much" and doesn't separate concerns. I think it would be better to have specific components handle specific events rather than try to make one big class that handles everything. \$\endgroup\$markspace– markspace2017年12月23日 22:29:38 +00:00Commented Dec 23, 2017 at 22:29
2 Answers 2
I think it's pretty good, though it looks a bit more like C code than java. You can compare it to my version of key tracking. Instead of the keys and keysLast arrays, I use an array of KeyState objects.
KeyState keyStates[] = new KeyState[NUM_KEYS];
public class KeyState {
public boolean down; // is this key pressed down right now?
public boolean downTrigger; // was this key first pressed down during this tick?
public long downNano; // nanoTime() that this key went to the down state (valid only if down is true)
public KeyState()
{
down = false;
downTrigger = false;
}
public void pressed()
{
down = true;
downTrigger = true;
downNano = System.nanoTime();
}
public void released()
{
down = false;
}
public void reset()
{
downTrigger = false;
}
}
This is more object oriented, though I use the more unusual convention of making member variables public, but with the understanding that they should only be read and not modified outside of the containing class. The more standard practice would be to make them private (or protected) and have getter functions.
To check if a key is pressed:
if (keyStates[KeyEvent.VK_W].down)
Cohesion & the Single Responsibility Principle
From Robert C. Martins book Clean Code, Chapter 10, Cohesion
Classes should have a small number of instance variables. Each of the methods of a class should manipulate one or more of those variables. In general the more variables a method manipulates the more cohesive that method is to its class. A class in which each variable is used by each method is maximally cohesive.
How Robert C. Martin describes the Single Responsibility Principle
Another wording for the Single Responsibility Principle is:
Gather together the things that change for the same reasons. Separate those things that change for different reasons.
If you think about this you’ll realize that this is just another way to define cohesion and coupling. We want to increase the cohesion between things that change for the same reasons, and we want to decrease the coupling between those things that change for different reasons.
Analise the Cohesion of Input
A class in which each variable is used by each method is maximally cohesive.
We can find no method with this property. Let's observe why:
private boolean isCtrlPressed = false; private boolean isShiftPressed = false; private boolean isAltPressed = false; private boolean isEscapePressed = false;
All these instance variables gets used together in the methods keyPressed
and keyReleased
. This means the belong some how together.
private boolean isWheelClicked = false; private boolean isRightClicked = false; private boolean isLeftClicked = false;
These instance variables have the same property and gets used in the methods mousePressed
and mouseReleased
.
These two instance variable groups do not interact together this means they have a low cohesion - this is a logical cohesion. You can read on Wikipedia about Logical Cohesion:
Logical cohesion is when parts of a module are grouped because they are logically categorized to do the same thing even though they are different by nature (e.g. grouping all mouse and keyboard input handling routines).