First game of Snake

Question

I'm making baby steps in Java programming and I just ended coding basic game: Snake. Could you share with me what is needed to improve in this code? What about OOP here?

GameBoard.java

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

public class GameBoard extends JFrame {
    //setting board size
    public static int boardWidth = 1000;
    public static int boardHeight = 800;

    private JPanel scorePanel;

    public static void main(String[] args) {
        new GameBoard();
    }

    public GameBoard() {
        super("Snake Game");
        this.setSize(boardWidth, boardHeight);
        this.setResizable(false);
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

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

            }

            @Override
            public void keyPressed(KeyEvent e) {
                if (e.getKeyCode() == KeyEvent.VK_LEFT) {
                    DrawingTheBoard.theSnake.changeDirection(Direction.LEFT);
                } else if (e.getKeyCode() == KeyEvent.VK_UP) {
                    DrawingTheBoard.theSnake.changeDirection(Direction.UP);
                } else if (e.getKeyCode() == KeyEvent.VK_RIGHT) {
                    DrawingTheBoard.theSnake.changeDirection(Direction.RIGHT);
                } else if (e.getKeyCode() == KeyEvent.VK_DOWN) {
                    DrawingTheBoard.theSnake.changeDirection(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, 1000, TimeUnit.MILLISECONDS); // updating board each 100MS

        this.setVisible(true);
    }
}

class RepaintTheBoard implements Runnable {
    private GameBoard theBoard;

    public RepaintTheBoard(GameBoard theBoard) {
        this.theBoard = theBoard;
    }

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

class DrawingTheBoard extends JComponent {
    static Snake theSnake = new Snake(GameBoard.boardWidth, GameBoard.boardHeight);
    static SnakeState theState = new SnakeState();
    private LinkedList<Point> body;
    private Point apple;

    public JLabel scoreLabel;
    public int score = 0;

    public DrawingTheBoard() {
        apple = new Apple().getPointOfApple();
        scoreLabel = new JLabel("Score: " + Integer.toString(score));
        scoreLabel.setFont(new Font("Serif", Font.PLAIN, 40));
    }

    public void paint(Graphics g) {
        body = theSnake.getBody(); // making copy of List from Snake to draw it on screen
        Graphics2D g2D = (Graphics2D) g;
        g2D.setBackground(Color.BLACK);
        g2D.fillRect(0, 0, getWidth(), getHeight());
        g2D.setPaint(Color.WHITE);

        for (Point partOfBody : body) {
            g2D.fillRect(partOfBody.getX(), partOfBody.getY(), 10, 10); // painting body of rectangles 10x10
        }

        theSnake.update(apple);

        // checking if is gameOver
        if (theState.checkIfSnakeIsDead(body)) {
            theSnake.restartSnake();
            score = 0;
            setScoreLabel(score);
        }

        if (theSnake.hasEaten()) {
            apple = new Apple().getPointOfApple(); // if snake has eaten apple creating new one
            theSnake.setHasEatenApple(false);
            score += 10;
            setScoreLabel(score); // adding 10 to score
        }
        g2D.fillRect(apple.getX(), apple.getY(), 10, 10);
    }

    public void setScoreLabel(int newScore) {
        scoreLabel.setText("Score: " + Integer.toString(newScore));
    }
}

Apple.java

import java.util.Random;

    public class Apple {
        private int bWidth = GameBoard.boardWidth;
        private int bHeight = GameBoard.boardHeight;
        private int randomXPos;
        private int randomYPos;
        private Point pointOfApple;

        Random r = new Random();
        public Apple(){
            randomXPos = r.nextInt((bWidth-10)/10)*10;
            randomYPos = r.nextInt((bHeight-110)/10)*10;
            pointOfApple = new Point(randomXPos,randomYPos);
        }
        public Point getPointOfApple(){
            return pointOfApple;
        }
        public int getRandomXPos(){
            return randomXPos;
        }
        public int getRandomYPos(){
            return randomYPos;
        }
    }

Snake.java

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

public class Snake extends Rectangle {
    private LinkedList<Point> body; // list holding points(x,y) of snake body
    private Direction headDirection;
    private boolean hasEatenApple = false;
    private int bWidth;
    private int bHeight;


    public Snake(int bWidth, int bHeight) {
        this.bWidth = bWidth;
        this.bHeight = bHeight;

        body = new LinkedList<>();
        restartSnake();

    }

    public LinkedList<Point> getBody() {
        return (LinkedList<Point>) body.clone();
    }

    public void restartSnake() {
        body.clear();

        //creating 3-part starting snake
        body.addFirst(new Point(this.bWidth / 2, this.bHeight / 2));
        body.add(new Point((this.bWidth / 2) - 10, this.bHeight / 2));
        body.add(new Point((this.bWidth / 2) - 20, this.bHeight / 2));
        this.headDirection = Direction.RIGHT;
    }

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

    //updating localisation of snake
    public void update(Point applePoint) {
        if (headDirection == Direction.DOWN) {
            addPartOfBody(0, 10, applePoint);
        } else if (headDirection == Direction.UP) {
            addPartOfBody(0, -10, applePoint);

        } else if (headDirection == Direction.LEFT) {
            addPartOfBody(-10, 0, applePoint);

        } else if (headDirection == Direction.RIGHT) {
            addPartOfBody(10, 0, applePoint);

        }
    }

    public void addPartOfBody(int x, int y, Point applePoint) {
        body.addFirst(new Point(body.get(0).getX() + x, body.get(0).getY() + y));

        //if new point of head of snake is point of apple don't reduce last part of snake
        if (body.get(0).getX() != applePoint.getX() || body.get(0).getY() != applePoint.getY()) {
            body.removeLast();
        } else {
            hasEatenApple = true;
        }
    }
    public boolean hasEaten(){
        return hasEatenApple;
    }
    public void setHasEatenApple(boolean hasEatenApple){
        this.hasEatenApple = hasEatenApple;
    }


}

SnakeState.java

import java.util.LinkedList;

public class SnakeState {
    private int bWidth = GameBoard.boardWidth;
    private int bHeight = GameBoard.boardHeight;
    private static boolean isDead = false;

    public boolean checkIfSnakeIsDead(LinkedList<Point> body) {

        //checking if head of snake is out of board
        if (body.get(0).getX() < 0 || body.get(0).getX() > bWidth-10
                || body.get(0).getY() < 0 || body.get(0).getY() > bHeight-110) {
            return true;
        }

        //checking if head of the snake have collision with others parts of his body
        for (int i = 1; i < body.size(); i++) {
            if (body.get(i).getX() == body.get(0).getX() && body.get(i).getY() == body.get(0).getY())
                return true;
        }
        return false;
    }

}

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;
        }
    },
    RIGHT {
        Direction opposite() {
            return LEFT;
        }
    },
    UP {
        Direction opposite() {
            return DOWN;
        }
    },
    DOWN {
        Direction opposite() {
            return UP;
        }
    };

    abstract Direction opposite();
}

Answer 1

General

You are quite fine with the separation of concerns. You identified the most important parts of the game and you gave them meaningful names.

As your approach is semantical I'd like to follow it and make some hints in which semantical direction the code will be even more plausible.

Finally I will make some more technical suggestions.

Game instance

Currently your Gameboard is responsible to start over again. You have to reset the snake, reset the score. What you are doing here is: You recycle a "game instance". In this simple case this is not that a problem. If you have complex objects you may forget reinitializing some of the values. This is error prone if you do not make this to a "big concept" (like a state pattern) OR your introduce "Game Instances" that you can dismiss if the "game is over".

Direction management

I think you did not handle the aspect "One direction change per tick". If someone enters the direction very fast there can be the problem that the snake is turning 180 degrees immediately.

To avoid this you have to change the semantic "changing the direction of a snake" to "make a direction change request". You should store the requested direction and evaluate it everytime your game makes a tick.

One other thing is, that your direction enum could be used to let polymorphism work to simplifiy your code.

...
LEFT {
    Direction opposite() {
        return RIGHT;
    }
    @Override
    int getX() {
        return -1;
    }
    @Override
    int getY() {
        return 0;
    }
}
...

Your update-method could look like this:

...
public void update(Point applePoint) {
   addPartOfBody(headDirection.getX() * 10, headDirection.getY() * 10, applePoint);
}
...

Apple

To get the semantic right: An apple does not place itself on the game board. It is placed by ... yeah ... that has to be identified. Maybe it is a game rule that has to be applied at the very beginning of a game and after the snake has eaten the apple. An apple can be eaten and placed only if the game makes a tick.

This is a game rule that will be applied to a concrete game instance at a tick timepoint.

At the moment the apple is nothing more than an empty shell to create a Point-Object. The semantic of an apple forgotton as soon as it was instantiated that is sad as you could pass it into methods to make things more explicit rather than passing around "abstract points".

Gameboard

You have encrypted the game board fields in UI calculation. My prefered way is to have such essential information stated clearly in the code like "the game board has 10x8 fields". Any other information should be derived from this.

Maybe you should introduce a 2D array of Gameboardelements as most gameboards have such a structure. Think about a chess board, connect four or Tic Tac Toe. The highest similarity to the reality is a 2D array to represent a game board.

Snake

The addPartOfBody-method can be reduced in visibility. Use the "private" modifier if you can as you show too much inner state to other objects. The goal should be to keep your public interface as small as possible.

The aspect "eating the apple" is really hidden and handled deep inside the snake. ;-). On the one hand I am kidding on the other hand I am serious because the only code artefact that reminds someone is the name of the variable "applePoint". I suggest to at least pass the apple. But that would not be my favorite way.

I would detect the apple collision outside the snake and let the snake eat the apple. If an apple was passed then the snake will grow. Something like this:

if (collides(snake, apple)) {
    Apple apple = takeAppleFromGameBoard();
    snake.eat(apple);
}

The eat-method:

public class Snake {
    ....
    public void eat(Apple apple) {
        if (apple != null) {
            grow();
        }
    }
    ....
}

This is only an idea. Keep the semantics right and your code will be more clear.

The trick is to identify some really clear statements and structures and develop the other aspects strictly around them.

Coupling

You have a strong coupling with the UI. My way to have as less coupling as possible is to start implementing the game without any UI. And if I say UI I also mean any visual representation of the game like console output/input.

If you restrict yourself to this methodological process of implementation you get at least some essential decoupling out of the box.

JFrame exit on close

I suggest to use DISPOSE_ON_CLOSE rather that using EXIT_ON_CLOSE. This forces you to think about proper shutdown mechanisms for your application. Using EXIT_ON_CLOSE is like "I don't care about application state". DISPOSE_ON_CLOSE preserves you the knowledge about all resources being used in your application and handle their shutdown properly.

Avoid multiple return statements

Even if the method is really small (and that's fine) you should consider to avoid multiple return statements. Return statements hinder you to apply refactorings like IDE supported "extract method".

Even there are other opinions I strictly follow the "only one exit" rule for the reasons to ease future refactorings. That does also apply to "break" and "continue".

Answer 2

@oopexpert has addressed the OOP issues, I'd just like to comment a bit about the style.

• You have some arbitrary values in there that do not mean much by themselves. Especially bHeight-110 and bWidth-10. It's not immediately clear what they mean, maybe you should consider using constants for 10 and 110 or (better IMHO), a method to get these values from the GameBoard class.
• You're using body.get(0) quite a lot, you might want to consider putting that in a variable, maybe something called head.
• I think it's more readable if you choose good function names and remove comments.

This:

        //checking if head of snake is out of board
        if (body.get(0).getX() < 0 || body.get(0).getX() > bWidth-10
                || body.get(0).getY() < 0 || body.get(0).getY() > bHeight-110) {
            return true;
        }

        //checking if head of the snake have collision with others parts of his body
        for (int i = 1; i < body.size(); i++) {
            if (body.get(i).getX() == body.get(0).getX() && body.get(i).getY() == body.get(0).getY())
                return true;
        }

Could become:

if (head.isOutOfBoard()) {
    return true;
}

if (head.collidesWithSomething()) {
    return true;
}

• This brings me to something I personally don't like, which is that I'd consider the GameBoard to be an object, like everything else. So, I think you should keep a GameBoard instance and run a game loop, instead of accessing the class statically. I think that the class that runs everything should be a generic App or Game class.

• After this, you could decouple the check from the fact that the collision is with the snake's own body, because what you actually want is to check that the snake collided with something that is not an apple. So, if you want to put walls in the middle of the board, you can just have a list of the "deadly" locations, which is updated with the position of the body. This would reduce the checkIfSnakeIsDead to a single for loop, because you could also put walls all around the board and remove the first check.

Something like:

if (head.collidesWith(gameBoard.getOccupiedLocations())) {
    return true;
}

Where gameBoard.getOccupiedLocations() returns a list of positions, which includes walls and the snake body.