2
\$\begingroup\$

A week ago I requested a review of my code for a game of Snake. First game of Snake I made some changes based on your answers and now I want to show you present code. Something else to modify here?

GameMain.java

     import javax.swing.*;

public class GameMain extends JFrame{
    public static void main(String[] args) {
        JFrame frame = new GameInstant();
        frame.setTitle("Snake Game");
        frame.setSize(1000,800);
        frame.setResizable(false);
        frame.setLocationRelativeTo(null);
        frame.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
        frame.setVisible(true);
    }
}

GameInstance.java

    import javax.swing.*;
import java.awt.*;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

public class GameInstant extends JFrame {
    private JPanel scorePanel;
    SnakeGame snakeGame = new SnakeGame();

    public GameInstant() {
        addKeyListener(new KeyListener() {
            @Override
            public void keyTyped(KeyEvent e) {

            }

            @Override
            public void keyPressed(KeyEvent e) {
                if (e.getKeyCode() == KeyEvent.VK_LEFT) {
                    snakeGame.storeDirectionOfSnake(Direction.LEFT);
                } else if (e.getKeyCode() == KeyEvent.VK_UP) {
                    snakeGame.storeDirectionOfSnake(Direction.UP);
                } else if (e.getKeyCode() == KeyEvent.VK_RIGHT) {
                    snakeGame.storeDirectionOfSnake(Direction.RIGHT);
                } else if (e.getKeyCode() == KeyEvent.VK_DOWN) {
                    snakeGame.storeDirectionOfSnake(Direction.DOWN);
                }
            }

            @Override
            public void keyReleased(KeyEvent e) {

            }
        });

        DrawingTheBoard gamePanel = new DrawingTheBoard();
        this.add(gamePanel, BorderLayout.CENTER);

        scorePanel = new JPanel();
        scorePanel.add(gamePanel.scoreLabel, BorderLayout.CENTER);
        this.add(scorePanel, BorderLayout.PAGE_END);

        ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(5);
        executor.scheduleAtFixedRate(new RepaintTheBoard(this), 0, snakeGame.getGameSpeed(), TimeUnit.MILLISECONDS);
    }
}

class RepaintTheBoard implements Runnable {
    private GameInstant theGame;

    public RepaintTheBoard(GameInstant theGame) {

        this.theGame = theGame;
    }

    public void run() {
        theGame.repaint();
    }
}

class DrawingTheBoard extends JComponent {
    public JLabel scoreLabel;
    private boolean inGame = false;
    private int score = 0;
    CellData[][] board;
    SnakeGame snakeGame = new SnakeGame();
    GameBoard gameBoard = new GameBoard();

    public DrawingTheBoard() {
        board = gameBoard.getBoard();
        scoreLabel = new JLabel("Score: " + score);
        scoreLabel.setFont(new Font("Serif", Font.PLAIN, 40));
    }

    public void paint(Graphics g) {
        Graphics2D g2D = (Graphics2D) g;
        g2D.setBackground(Color.BLACK);
        g2D.fillRect(0, 0, getWidth(), getHeight());
        update();

        for (int i = 0; i < gameBoard.getxCells(); i++) {
            for (int j = 0; j < gameBoard.getyCells(); j++) {
                if (board[i][j] == CellData.APPLE || board[i][j] == CellData.SNAKE) {
                    g2D.setPaint(Color.WHITE);
                    g2D.fillRect(i * 10, j * 10, 10, 10);
                } else if (board[i][j] == CellData.WALL) {
                    g2D.setPaint(Color.RED);
                    g2D.fillRect(i * 10, j * 10, 10, 10);
                }
            }
        }
        if (snakeGame.hasEatenApple()) {
            score += 10;
            scoreLabel.setText("Score: " + Integer.toString(score));
        } else if (snakeGame.isDead()) {
            score = 0;
            scoreLabel.setText("Score: " + Integer.toString(score));
        }
    }

    public void update() {
        if (inGame == false) {
            snakeGame.initializeGame();
            inGame = true;
        }
        snakeGame.changeSnakeDirection();
        snakeGame.updateSnake();
        if (snakeGame.snakeIsDead()) {
            snakeGame.removeSnake();
            snakeGame.initializeGame();
        }
        snakeGame.updateApple();
        snakeGame.updateBoard();
    }
}

SnakeGame

    import java.util.LinkedList;

public class SnakeGame {
    private int gameSpeed = 100;
    private LinkedList<Point> body;
    private Point head;
    private static boolean eatenApple = false;
    private static boolean isDead = false;
    private static Direction snakeDirection;
    Snake theSnake = new Snake();
    Apple theApple = new Apple();
    GameBoard board = new GameBoard();

    public SnakeGame() {
    }

    public void initializeGame() {
        board.cleanBoard();
        theSnake.createSnake(board.getxCells() / 2, board.getyCells() / 2);
        theApple.createNewApple();
        addAppleToGameBoard();
    }

    public boolean collidesWith(CellData cellData) {
        body = theSnake.getBody();
        head = body.get(0);
        CellData cell = board.getBoard()[head.getX()][head.getY()];
        return (cell == cellData);

    }

    public boolean snakeIsDead() {
        if (collidesWith(CellData.WALL)
                || collidesWith(CellData.SNAKE)) {
            isDead = true;
            return true;
        } else {
            isDead = false;
            return false;
        }
    }

    public void takeAppleFromGameBoard() {
        board.setDataCell(theApple.getRandomXPos(), theApple.getRandomYPos(), CellData.EMPTY);
    }

    public void addAppleToGameBoard() {
        board.setDataCell(theApple.getRandomXPos(), theApple.getRandomYPos(), CellData.APPLE);
    }

    public void updateApple() {

        if (collidesWith(CellData.APPLE)) {
            takeAppleFromGameBoard();
            theSnake.eat();
            theApple.createNewApple();
            eatenApple = true;
        } else {
            eatenApple = false;
        }
    }

    public void storeDirectionOfSnake(Direction direction) {
        snakeDirection = direction;
    }

    public void changeSnakeDirection(){
        if (snakeDirection != null) {
            theSnake.changeDirection(snakeDirection);
        }
    }
    public void addSnakeToBoard() {
        body = theSnake.getBody();
        for (int i = 0; i < body.size(); i++) {
            board.setDataCell(body.get(i).getX(), body.get(i).getY(), CellData.SNAKE);
            board.setDataCell(theSnake.getTailCell().getX(), theSnake.getTailCell().getY(), CellData.EMPTY);
        }
    }

    public void updateSnake() {
        theSnake.update();
    }

    public void updateBoard(){
        addAppleToGameBoard();
        addSnakeToBoard();
    }

    public void removeSnake() {
        body = theSnake.getBody();
        theSnake.clearBody();
        for (int i = 0; i < body.size(); i++) {
            board.setDataCell(body.get(i).getX(), body.get(i).getY(), CellData.EMPTY);
        }
    }
    public int getGameSpeed() {
        return gameSpeed;
    }

    public boolean hasEatenApple() {
        return eatenApple;
    }

    public boolean isDead() {
        return isDead;
    }
}

GameBoard.java

public class GameBoard {
    private int boardWidth = 1000;
    private int boardHeight = 700;
    private int xCells = boardWidth / 10;
    private int yCells = boardHeight / 10;
    private static CellData board[][];

    public GameBoard() {
        board = new CellData[xCells][yCells];
    }

    public void cleanBoard() {
        for (int i = 0; i < xCells; i++) {
            board[i][0] = CellData.WALL;
        }
        for (int i = 0; i < xCells; i++) {
            board[i][yCells - 1] = CellData.WALL;
        }
        for (int j = 0; j < yCells; j++) {
            board[0][j] = CellData.WALL;
        }
        for (int j = 0; j < yCells; j++) {
            board[xCells - 1][j] = CellData.WALL;
        }
        for (int i = 1; i < xCells - 1; i++) {
            for (int j = 1; j < yCells - 1; j++) {
                board[i][j] = CellData.EMPTY;
            }
        }
    }

    public void setDataCell(int x, int y, CellData cellData) {
        board[x][y] = cellData;
    }

    public CellData[][] getBoard() {
        return board;
    }

    public int getxCells() {
        return xCells;
    }

    public int getyCells() {
        return yCells;
    }


}

Apple.java

    import java.util.Random;

public class Apple {
    private int randomXPos;
    private int randomYPos;
    Random r = new Random();
    GameBoard board = new GameBoard();

    public Apple(){
    }

    public void createNewApple(){
        randomXPos = r.nextInt(board.getxCells()-2)+1;
        randomYPos = r.nextInt(board.getyCells()-2)+1;
    }

    public int getRandomXPos(){
        return randomXPos;
    }

    public int getRandomYPos(){
        return randomYPos;
    }

}

Snake.java

import java.awt.*;
import java.util.LinkedList;

public class Snake{
    private LinkedList<Point> body; // list holding points(x,y) of snake body
    private Point head;
    private static Direction headDirection;
    private static Point tailCell;
    private static boolean hasEatenApple = false;

    public Snake() {
        body = new LinkedList<>();
    }

    public void createSnake(int x, int y) {

        //creating 3-part starting snake
        body.addFirst(new Point(x,y));
        body.add(new Point(x - 1, y));
        body.add(new Point(x - 2, y));
        headDirection = Direction.RIGHT;
        tailCell = body.getLast();

    }

    public void clearBody(){body.clear();
    }
    public void changeDirection(Direction theDirection) {
        if (theDirection != headDirection.opposite())
            this.headDirection = theDirection;
    }

    //updating localisation of snake
    public void update() {
        addPartOfBody(headDirection.getX(), headDirection.getY());

    }

    private void addPartOfBody(int x, int y) {
        head = body.get(0);
        body.addFirst(new Point(head.getX() + x, head.getY() + y));
        tailCell = body.getLast();
        if (hasEatenApple == false) {
            body.removeLast();
        } else {
            hasEatenApple = false;
        }
    }
    public LinkedList<Point> getBody() {
        return (LinkedList<Point>) body.clone();
    }
    public Point getTailCell(){return tailCell;}
    public void eat() {
        hasEatenApple = true;
    }
}

Point.java

public class Point {
    private int x;
    private int y;

    public Point(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }
}

Direction.java

public enum Direction {
    LEFT {
        Direction opposite() {
            return RIGHT;
        }
        int getX(){
            return -1;
        }
        int getY(){
            return 0;
        }
    },
    RIGHT {
        Direction opposite() {
            return LEFT;
        }
        int getX(){
            return 1;
        }
        int getY(){
            return 0;
        }
    },
    UP {
        Direction opposite() {
            return DOWN;
        }
        int getX(){
            return 0;
        }
        int getY(){
            return -1;
        }
    },
    DOWN {
        Direction opposite() {
            return UP;
        }
        int getX(){
            return 0;
        }
        int getY(){
            return 1;
        }
    };

    abstract Direction opposite();
    abstract int getX();
    abstract int getY();
}

CellData.java

public enum CellData {
    EMPTY, SNAKE, APPLE, WALL;
}
\$\endgroup\$
2
\$\begingroup\$

Suggestions

  1. Currently, it seems as if you would like the game to run in full-screen mode, but what about the few people in the world still having 1280x720 or lower displays on their systems? 1000x800 will run out of vertical screen space on such displays.

    If you want to make a proper full-screen display of your JFrame, try the following code from this SO answer: JFrame in full screen Java:

    frame.setExtendedState(JFrame.MAXIMIZED_BOTH); 
    frame.setUndecorated(true);
    

    Use this just before the .setVisible(true) call.

    You should also take a look at this, the Java Exclusive Full-Screen mode API. That should help when you want to get fullscreen properly (which the previous suggestion essentially is not, as it's not exclusive).

  2. You already import java.awt. Why not use it's Point class instead of rolling your own? It works pretty much the same, so it should be a drop-in replacement at this stage.

  3. Get your boardWidth and boardHeight parameters from the host JFrame as parameters to your GameBoard constructor, and move all field initialization there. This should make your code more flexible against different resolutions.

  4. You don't use the java.awt.* you imported for Snake.java. you can safely get rid of it.

  5. Any reason why eatenApple and isDead are static in SnakeGame? I don't think that they need to be.

  6. Division of responsibility:

I feel that spawning an apple on the board should be the responsibility of the board, not the apple. Also, maintaining the state of a snake should be the responsibility of the snake, not the game logic. So, createNewApple() should belong to GameBoard and isDead() should belong to Snake, along with the previously mentioned variables (point 5). If you absolutely need to, you could expose these values using getters in SnakeGame.

The next is a tricky point, what you've done is correct, just letting you know why you shouldn't change it in the future.

  1. Instead of using LinkedList for representing the points of the body of the snake, never use a java.util.ArrayList, even if you do an ensureCapacity(xCells*yCells-2*(xCells+yCells)) call on the ArrayList object when initializing it to prevent reallocations (xCells*yCells2*(xCells+yCells) is the maximum length of the snake).

    A linked list takes O(n) time to remove it's last element if it's a singly linked list with only a head pointer, whereas for an array it is always an O(1) operation. Now java.util.LinkedList is a doubly linked list for which deletion of the last element can be done in O(1) time, so in this case the time complexity is not an obvious saving, however, when it comes to adding an element to the head of the list, the story is completely different. Then, ArrayList takes O(n) time while LinkedList takes O(1).

    TL;DR Keep using LinkedList.

Style

Indentation

I'm sure this point was raised in answers to your previous question, but your indentation, linebreaks and braces are inconsistent. Try to use an editor or IDE capable of autoformatting to help you in this. Try to follow one style of indentation and braces and be consistent, it greatly improves the readability of your code.

Naming

Maybe you're autogenerating getters and setters, but take care, the API they expose is not evident from their names. In Apple.java, getRandomxPos() & getRandomyPos() seem to return only a particular x (or y, respectively) position, which is predetermined. Drop the Random in their names, it makes no sense as part of the API. Similarly for jFrame. That's not really representative of what jFrame's purpose is. You have gotten away with GameBoard and SnakeGame thanks to the class names, but try to indicate the purpose of a variable via its name.

\$\endgroup\$
  • \$\begingroup\$ If createNewApple() will belong to GameBoard.java should I delete class Apple.java? \$\endgroup\$ – Aro400 Nov 22 '16 at 18:14
  • \$\begingroup\$ @Aro400 since doing that makes Apple.javaessentially useless except as the container for the position of the apple (which can also be modelled by the Point class), you could delete that for and just keep something like spawnRandomApple() instead of createNewApple() in GameBoard.java, and store this location in a Point variable called, say, appleLocation in GameBoard.java \$\endgroup\$ – Tamoghna Chowdhury Nov 22 '16 at 18:24
  • \$\begingroup\$ Ok, thanks. I did that in GameBoard.java like you said. \$\endgroup\$ – Aro400 Nov 22 '16 at 18:47

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.