1
\$\begingroup\$

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;
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Dec 23, 2017 at 21:59
\$\endgroup\$
1
  • 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\$ Commented Dec 23, 2017 at 22:29

2 Answers 2

1
\$\begingroup\$

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)
answered Mar 20, 2019 at 17:51
\$\endgroup\$
1
\$\begingroup\$

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).

answered Mar 21, 2019 at 8:26
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.