2
\$\begingroup\$

I thought of a program where you move a square with the arrow keys. I created it and asked a question about it on Stack Overflow.

I copied the last code block of the accepted answer and tried to understand it. I think I do now. Now I wanted to add something that would check if the square would move off the screen. I added that myself as seen here:

Code inside Square class that replaces the move method of the copied code:

public void move(Direction dir) {
    if(!(x + step * dir.getIncrX() > GamePanel.getWIDTH() - w) && !(x + step * dir.getIncrX() < 0))
        x += step * dir.getIncrX();
    if(!(y + step * dir.getIncrY() > GamePanel.getHEIGHT() - h) && !(y + step * dir.getIncrY() < 0))
        y += step * dir.getIncrY();
}

To achieve this I had to make the constants WIDTH and HEIGHT static and create getters for them. Is the way I created this collision detection good practise? and should the getters for the constants be called getWidth and getHeight istead of getWIDTH and GetHEIGHT?

Full code:

import java.awt.*;
import javax.swing.*;
import java.awt.event.*;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.Map;

public class GamePanel extends JPanel {
    private static final int ANIMATION_DELAY = 15;
    private static final int HEIGHT = 500;
    private static final int WIDTH = 500;
    private Square square;
    private EnumMap<Direction, Boolean> dirMap = new EnumMap<>(Direction.class);
    private Map<Integer, Direction> keyToDir = new HashMap<>();

    public GamePanel() {
        for (Direction dir : Direction.values()) {
            dirMap.put(dir, false);
        }
        keyToDir.put(KeyEvent.VK_UP, Direction.UP);
        keyToDir.put(KeyEvent.VK_DOWN, Direction.DOWN);
        keyToDir.put(KeyEvent.VK_LEFT, Direction.LEFT);
        keyToDir.put(KeyEvent.VK_RIGHT, Direction.RIGHT);
        setKeyBindings();
        setBackground(Color.white);
        setPreferredSize(new Dimension(getWIDTH(), getHEIGHT()));
        setFocusable(true);
        square = new Square();
        Timer animationTimer;
        animationTimer = new Timer(ANIMATION_DELAY, new AnimationListener());
        animationTimer.start();
        square.setStep(5);
    }

    public static int getHEIGHT() {
        return HEIGHT;
    }

    public static int getWIDTH() {
        return WIDTH;
    }

    private void setKeyBindings() {
        int condition = WHEN_IN_FOCUSED_WINDOW;
        final InputMap inputMap = getInputMap(condition);
        final ActionMap actionMap = getActionMap();
        boolean[] keyPressed = { true, false };
        for (Integer keyCode : keyToDir.keySet()) {
            Direction dir = keyToDir.get(keyCode);
            for (boolean onKeyPress : keyPressed) {
                boolean onKeyRelease = !onKeyPress;
                KeyStroke keyStroke = KeyStroke.getKeyStroke(keyCode, 0, onKeyRelease);
                Object key = keyStroke.toString();
                inputMap.put(keyStroke, key);
                actionMap.put(key, new KeyBindingsAction(dir, onKeyPress));
            }
        }
    }

    public void paintComponent(Graphics g) {
        super.paintComponent(g);
        square.display(g);
    }

    private class AnimationListener implements ActionListener {
        @Override
        public void actionPerformed(ActionEvent e) {
            boolean repaint = false;
            for (Direction dir : Direction.values()) {
                if (dirMap.get(dir)) {
                    square.move(dir);
                    repaint = true;
                }
            }
            if (repaint)
                repaint();
        }
    }

    private class KeyBindingsAction extends AbstractAction {
        private Direction dir;
        boolean pressed;

        public KeyBindingsAction(Direction dir, boolean pressed) {
            this.dir = dir;
            this.pressed = pressed;
        }

        @Override
        public void actionPerformed(ActionEvent evt) {
            dirMap.put(dir, pressed);
        }
    }

    private static void createAndShowGUI() {
        GamePanel gamePanel = new GamePanel();
        JFrame frame = new JFrame("GamePanel");
        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        frame.getContentPane().add(gamePanel);
        frame.pack();
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
        gamePanel.requestFocusInWindow();
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                createAndShowGUI();
            }
        });
    }
}

enum Direction {
    UP(0, -1), DOWN(0, 1), LEFT(-1, 0), RIGHT(1, 0);
    private int incrX;
    private int incrY;

    Direction(int incrX, int incrY) {
        this.incrX = incrX;
        this.incrY = incrY;
    }

    public int getIncrX() {
        return incrX;
    }

    public int getIncrY() {
        return incrY;
    }
}

class Square {
    private int x = 0;
    private int y = 0;
    private int w = 20;
    private int h = w;
    private int step = 1;
    private Color color = Color.red;

    public void display(Graphics g) {
        Graphics2D g2d = (Graphics2D) g.create();
        g2d.setColor(color);
        g2d.fillRect(x, y, w, h);
        g2d.dispose();
    }

    public void setStep(int step) {
        this.step = step;
    }

    public void move(Direction dir) {
        if(!(x + step * dir.getIncrX() > GamePanel.getWIDTH() - w) && !(x + step * dir.getIncrX() < 0))
            x += step * dir.getIncrX();
        if(!(y + step * dir.getIncrY() > GamePanel.getHEIGHT() - h) && !(y + step * dir.getIncrY() < 0))
            y += step * dir.getIncrY();
    }
}
\$\endgroup\$
2
\$\begingroup\$

Change the visibility of WIDTH and HEIGHT to public, so you can call them by GamePanel.WIDTH and GamePanel.HEIGHT. Because for static final variables you never create Getters! And you should change your code like this to improve it's performance:

public void move(Direction dir) {
    int newX = x + step * dir.getIncrX(), newY = y + step * dir.getIncrY();
    if (!(newX > GamePanel.WIDTH - w) && !(newX < 0)) x = newX;
    if (!(newY > GamePanel.HEIGHT - h) && !(newY < 0)) y = newY;
}
\$\endgroup\$
  • \$\begingroup\$ So should I make all static final variables to public by default? And thanks. \$\endgroup\$ – The Coding Wombat May 14 '15 at 18:00
  • 1
    \$\begingroup\$ You can, but if you use a variable just in the same class you can make them private static final. But if you use them in another class, you should make them public static final instead of a Getter. \$\endgroup\$ – Datagrammar May 14 '15 at 18:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.