6
\$\begingroup\$

I started coding in August 2018, and I was wondering on how I can improve as a programmer. I'm more concerned about coding conventions. Particularly, how can I improve the structure of my code, organization, comments, variables, formatting, etc. Any other feedback is welcome and appreciated as well.

I completed a Snake game with the following classes. The code can also be viewed here.

Body

package kane.game.snake;

import kane.game.map.Block;

/**
 * This class is a linked list of the bodies of a snake. The body index should be right before the previous node,
 * depending on the direction.
 */
public class Body extends Block {
    private Direction dir;
    /**
     * Initializes the body at the following index.
     * @param row Row index of the body.
     * @param col Col index of the body.
     */
    public Body(int row, int col){
        super(row, col);
    }

    /**
     * Returns the direction the snake is heading.
     * @return the direction the snake is heading.
     */
    public Direction getDir(){
        return dir;
    }

    /**
     * Sets the direction the snake is heading.
     * @param dir the direction the snake is heading.
     */
    public void setDir(Direction dir){
        this.dir = dir;
    }

}

Direction

package kane.game.snake;

/**
 * This enum represents the direction of a snake block.
 */
public enum Direction{
    UP, DOWN, RIGHT, LEFT;
}

Snake

package kane.game.snake;

import java.util.ArrayList;

/**
 * This class represents the snake. Snake contains the head of the snake that the user controls.
 */
public class Snake {
    private ArrayList<Body> snake;

    /**
     * Initializes the snake with a head, which the starting index is at the center of the map.
     * @param width
     * @param height
     */
    public Snake(int width, int height){
        snake = new ArrayList<>();
        snake.add(new Body(height/2, width/2));
    }

    /**
     * Returns the first index of {@code snake}.
     * @return the {@code head} of {@code snake}.
     */
    public Body getHead(){
        return snake.get(0);
    }

    /**
     * Returns the {@code snake}
     * @return the {@code snake}
     */
    public ArrayList<Body> getSnake(){
        return snake;
    }
    /**
     * Returns the size of the snake.
     * @return size of the snake.
     */
    public int size(){
        return snake.size();
    }

    /**
     * Returns the tail of the snake.
     * @return The last index of {@code snake}.
     */
    public Body getTail(){
        return snake.get(snake.size()-1);
    }

    /**
     * Adds a body to the snake.
     */
    public void appendBody(){
        Body tail = getTail();
        //The bodies are added depending on which direction the tail is heading.
        if(tail.getDir()==Direction.UP){
            snake.add(new Body(tail.getRow(), tail.getCol()-1));
            getTail().setDir(tail.getDir());
        }
        if(tail.getDir()==Direction.DOWN){
            snake.add(new Body(tail.getRow(), tail.getCol()+1));
            getTail().setDir(tail.getDir());
        }
        if(tail.getDir()==Direction.RIGHT){
            snake.add(new Body(tail.getRow()+1, tail.getCol()));
            getTail().setDir(tail.getDir());
        }
        if(tail.getDir()==Direction.LEFT){
            snake.add(new Body(tail.getRow()-1, tail.getCol()));
            getTail().setDir(tail.getDir());
        }
    }

    /**
     * Returns whether or not the snake collided with itself.
     * @return True if it collided with itself, false otherwise.
     *
     */
    public boolean isCollision(){
        for(Body b: snake){
            if(getHead().getRow()==b.getRow()&&getHead().getCol()==b.getCol()&&b!=getHead()){
                return true;
            }
        }
        return false;
    }

    /**
     * Moves the snake along.
     */
    public void passIndex(){
        for(int i = size()-1; i>0; i--){
            snake.get(i).setRow(snake.get(i-1).getRow());
            snake.get(i).setCol(snake.get(i-1).getCol());
        }
    }

}

Map

package kane.game.map;

import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
import javafx.scene.layout.GridPane;
import kane.game.snake.Body;
import kane.game.snake.Snake;

import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.util.Random;

/**
 * The class represents the game map of snake. The map includes properties such as {@code width}, {@code height},
 * {@code user}, and {@code apple}. It also holds the visual aspects of the game.
 */
public class Map {
    private ImageView[][] map;
    private int width;
    private int height;
    private Snake user;
    private Apple apple;
    private static Image whiteblock;
    private static Image snakeblock;
    private static Image appleblock;

    /**
     * This constructor initializes the map with a certain width and height. It take's in a GridPane argument, and
     * each index of this GridPane is set to be a background block ({@code whiteblock}). Additionally, the head of a
     * snake block is placed at the center, and an {@code apple} block is randomly placed on the map.
     * @param root
     * @param width
     * @param height
     */
    public Map(GridPane root, int width, int height){
        //Initializing whiteblock to a simple white square, snakeblock to be a simple red square,
        //and appleblock to be a red square.
        try {
            whiteblock = new Image(new FileInputStream("resources/whitesquare.jpg"));
            snakeblock = new Image(new FileInputStream("resources/blacksquare.png"));
            appleblock = new Image(new FileInputStream("resources/redsquare.jpg"));
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        }
        user = new Snake(width, height);
        //Apple placed at a random index.
        apple = new Apple(0,0);
        apple.placeApple(width,height);

        this.width = width;
        this.height = height;

        map = new ImageView[width][height];
        //Sets each GirdPane block to the corresponding 2d ImageView Array, map.
        for(int row = 0; row<width; row++){
            for(int col = 0; col<height; col++){
                //Initializes a new ImageView for each index.
                map[row][col] = new ImageView(whiteblock);
                //Sets the apple block on the GridPane. Overrides the background block.
                if(row==apple.getRow()&&col==apple.getCol()){
                    map[row][col].setImage(appleblock);
                }
                //Sets the head of the snake on the GridPane. Overrides the background block.
                if(row==user.getHead().getRow()&&col==user.getHead().getCol()){
                    map[row][col].setImage(snakeblock);
                }
                //Makes sure the dimensions of each index in GridPane is reasonable. 50x50 pixels
                map[row][col].setFitHeight(50);
                map[row][col].setFitWidth(50);
                //Adds each ImageView to the GridPane
                root.add(map[row][col], row, col);
            }
        }
    }

    /**
     * Returns the user snake.
     * @return {@code user}. The Snake object for this class.
     */
    public Snake getUser(){
        return user;
    }

    /**
     * Returns {@code apple}
     * @return The Apple object for this class.
     */
    public Apple getApple(){
        return apple;
    }

    /**
     * Returns the {@code width} of the map.
     * @return  {@code width} of the map.
     */
    public int getWidth(){
        return width;
    }
    /**
     * Returns the {@code height} of the map.
     * @return  {@code height} of the map.
     */
    public int getHeight(){
        return height;
    }

    /**
     * Updates the map by resetting each ImageView index in the array {@code map}. Updates visuals. This method should
     * be used after any change to a block.
     */
    public void updateMap(){
        for(int row = 0; row<width; row++){
            for(int col = 0; col<height; col++){
                //setImage instead of initializing is used for better memory usage.
                map[row][col].setImage(whiteblock);
                //Sets the apple block on the GridPane. Overrides the background block.
                if(row==apple.getRow()&&col==apple.getCol()){
                    map[row][col].setImage(appleblock);
                }
                for(int i = 0; i<user.size(); i++){
                    if(user.getSnake().get(i).getRow()==row&&user.getSnake().get(i).getCol()==col){
                        map[row][col].setImage(snakeblock);
                    }
                }
                //Makes sure the dimensions of each index in GridPane is reasonable. 50x50 pixels
                map[row][col].setFitHeight(50);
                map[row][col].setFitWidth(50);
                //Adds each ImageView to the GridPane
            }
        }
    }

}

Block

package kane.game.map;

/**
 * This class represents any block that would be in a snake game.
 */
public class Block {
    private int row;
    private int col;

    /**
     * Initializes the {@code row} and {@code col} of the block.
     * @param row row index of the block
     * @param col column index of the block
     */
    public Block(int row, int col){
        this.row = row;
        this.col = col;
    }

    /**
     * Returns the {@code row} index of the block.
     * @return Returns the {@code row} index of the block.
     */
    public int getRow(){
        return row;
    }

    /**
     * Sets the {@code row} index of the block.
     * @param row {@code row} index.
     */
    public void setRow(int row){
        this.row = row;
    }
    /**
     * Returns the {@code col} index of the block.
     * @return Returns the {@code col} index of the block.
     */
    public int getCol(){
        return col;
    }
    /**
     * Sets the {@code col} index of the block.
     * @param col {@code col} index.
     */
    public void setCol(int col){
        this.col = col;
    }
}

Apple

package kane.game.map;

import java.util.Random;

/**
 * This class represents a block that represents an apple.
 */
public class Apple extends Block {
    /**
     * Initializes the apple at the specified location.
     * @param row Row index of where apple will be placed.
     * @param col Col index of where apple will be placed.
     */
    public Apple(int row, int col) {
        super(row, col);
    }

    /**
     * Randomly places the apple at an index.
     * @param height Height of the map.
     * @param width Width of the map.
     */
    public void placeApple(int height, int width){
        Random rand = new Random();
        int appleRow = rand.nextInt(height);
        int appleCol = rand.nextInt(width);
        setRow(appleRow);
        setCol(appleCol);
    }
}

Movement

package kane.game.controls;

import javafx.scene.control.Label;
import kane.game.map.Map;
import kane.game.snake.Body;
import kane.game.snake.Direction;

import java.util.TimerTask;

/**
 * This class represents how the snake visually moves.
 */
public class Movement extends TimerTask {
    private Map map;
    private Body head;

    /**
     * Passes in the map to be referenced.
     * @param map map that is to be referenced.
     */
    public Movement(Map map){
        this.map =  map;
        this.head = map.getUser().getHead();
    }

    /**
     * Snake moves based off of the direction it is facing. The direction is changed with {@code UserInput} controls.
     */
    @Override
    public void run() {
        //Checks if the snake is out of bounds of the map. You lose if so.
        if(head.getRow()<0||head.getRow()>=map.getWidth()||head.getCol()<0||head.getCol()>=map.getHeight()){
            System.exit(0);
        }
        //Checks if the snake collides with itself. You lose if so.
        if(map.getUser().isCollision()){
            System.exit(0);
        }
        //When the snake eats an apple. Snake grows in size if so.
        if(map.getApple().getRow()==head.getRow()&&map.getApple().getCol()==head.getCol()) {
            map.getUser().appendBody();
            map.getApple().placeApple(map.getWidth(), map.getHeight());
            map.updateMap();
        }
        //Moves snake up.
        if(head.getDir()== Direction.UP){
            map.getUser().passIndex();
            head.setCol(head.getCol()-1);
            map.updateMap();
        }
        //Moves snake down.
        if(head.getDir()== Direction.DOWN){
            map.getUser().passIndex();
            head.setCol(head.getCol()+1);
            map.updateMap();
        }
        //Moves snake to the right.
        if(head.getDir()== Direction.RIGHT){
            map.getUser().passIndex();
            head.setRow(head.getRow()+1);
            map.updateMap();
        }
        //Moves snake to the left
        if(head.getDir()== Direction.LEFT){
            map.getUser().passIndex();
            head.setRow(head.getRow()-1);
            map.updateMap();
        }

    }
}

UserInput

package kane.game.controls;

import javafx.event.EventHandler;
import javafx.scene.input.KeyEvent;
import kane.game.map.Map;
import kane.game.snake.Body;
import kane.game.snake.Direction;
import kane.game.snake.Snake;

import java.util.Timer;
import java.util.TimerTask;

import static javafx.scene.input.KeyCode.*;

/**
 * This class represents the controls for a game of snake.
 */
public class UserInput implements EventHandler<KeyEvent> {
    private Body head;
    private Map map;

    /**
     * Passes in the head to reference for the direction.
     * @param head Head object to use to get the direction.
     */
    public UserInput(Map map, Body head){
        this.head = head;
        this.map = map;
    }

    /**
     * Controls of the game. Up, down, right, and left arrow keys are used for movement.
     * @param event the event that is being listened.
     */
    @Override
    public void handle(KeyEvent event) {
        //Each input is checked if it inputs the direct opposite direction. For example, if going up, you can not
        //switch direction to down. (Unless size of snake is 1).
        if(event.getCode()==UP){
            if(map.getUser().size()>1&&head.getDir()==Direction.DOWN){

            }else {
                head.setDir(Direction.UP);
            }
            }
        if(event.getCode()==DOWN){
            if(map.getUser().size()>1&&head.getDir()==Direction.UP){

            }else {
                head.setDir(Direction.DOWN);
            }
        }
        if(event.getCode()==RIGHT){
            if(map.getUser().size()>1&&head.getDir()==Direction.LEFT){

            }else {
                head.setDir(Direction.RIGHT);
            }
        }
        if(event.getCode()==LEFT){
            if(map.getUser().size()>1&&head.getDir()==Direction.RIGHT){

            }else {
                head.setDir(Direction.LEFT);
            }
        }
    }
}

Driver

package kane.game;

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.layout.GridPane;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
import kane.game.controls.Movement;
import kane.game.controls.UserInput;
import kane.game.map.Map;

import java.util.Timer;

/**
 * This is the driver class for a game of snake.
 *
 * Author: Kane Du
 * Last Edited: 3/17/19
 */
public class Driver extends Application {
    private static final int SNAKE_SPEED = 100;

    /**
     * Launches a JavaFX application.
     * @param args Command line arguments.
     */
    public static void main(String[] args) {
        launch(args);
    }

    /**
     * Creates the snake game window.
     * @param primaryStage the stage
     */
    @Override
    public void start(Stage primaryStage) {
        Stage window = primaryStage;
        window.setTitle("Snake");

        VBox root = new VBox();
        Label score = new Label("hello");


        //The layout that will be compared to an ImageView array.
        GridPane grid = new GridPane();

        //Initializing map for snake.
        Map map = new Map(grid, 21, 21);

        root.getChildren().addAll(score,grid);
        root.setVgrow(grid, Priority.ALWAYS);

        UserInput userInput = new UserInput(map, map.getUser().getHead());

        Scene scene = new Scene(root);
        scene.setOnKeyPressed(userInput);

        //Timer allows the snake to continue to move without a key press.
        Timer timer = new Timer();
        Movement movement = new Movement(map);
        timer.schedule(movement, 0, SNAKE_SPEED);

        window.setScene(scene);
        window.show();

    }
}
\$\endgroup\$
3
\$\begingroup\$

This is a very good for a few months of experience! You're on the right track with trying to separate concepts out into objects, but I think you made some strange decisions in your separating of concerns that I'll call out in addition to some stylistic things. Also decent documentation; keep that up!

  • Java is kind of verbose. This includes package names. Typically packages are named (com|org|net).yourdomain.yourproject (ex org.apache.commons). Now, since your package isn't meant to be used by anyone else, you can get away with this naming scheme. But, that doesn't mean you shouldn't follow the convention.
  • Your inheritance is strange. The chain for Snake is: Snake is composed of Body (which has a Direction) which is a Block. The chain for Apple is: Apple is a Block. While I see what you were trying to do here, the separation is odd. The Block superclass (which should be abstract, instantiating a Block means nothing) just encapsulates position. If block also handled drawing (since in the original snake, the snake's body and apples were just single pixels on a screen), this would make more sense. But, in this case, I'd recommend composition over inheritance. Create an immutable Point and give an Apple a Point member variable (instead of extending it). Instead of having the Body class, just give the Snake a ordered collection of points that form its body (more on both of these later).
  • You do not need to redefine a constructor if all it does is call super with the same args.
  • Map conflates two concerns: holding the game state and rendering the game state. Ideally you'd separate this or push drawing concerns into the objects themselves.
  • Never e.printStackTrace() outside of development. You'd want to display a nice error message. You can achieve this by letting the exception bubble up and catching it at a layer that can show UI. An editor like IntelliJ will be able to help with this because it can show you the path that an exception will bubble up (by making you add throws clauses to function definitions in Java 8).
  • placeApple should not be a method on Apple. If you were going to do that, call it randomizePosition, because that's what it actually does. But I'd recommend against that. In general, immutability is much easier to reason about. We'll discuss this in detail later. (In the end, placeApple should be a method of Map)
  • Movement is a strange class. I understand it ties into your game loop. I'd call it GameTickHandler, because it handles what actions should be taken every game "tick." I also wouldn't use java.util.TimerTask. I believe you have potential data races because TimerTask runs in a separate thread. In general, there's a more common pattern for games that I'll advocate for below.
  • Movement contains a lot of logic that should be in Direction (particularly the end of run). If you have an immutable Point then you could have a method movedTowards(Direction d) which returns a new Point moved in that direction. You can test this separately without having to deal with all of the complexities in Movement at all.
  • Don't System.exit(0)! Display some useful message to your user. The pattern I'll advocate that you use (mentioned above, more detail later) will make this easy.
  • How you check for collisions is strange. I'm imagining an API that is much clearer and separates concerns better. Give snake two methods intersectSelf and intersect(Apple a). This way only a snake decides what these mean. And if you need to check these multiple times, you don't have to duplicate the "head is touching a part of the body" logic.
  • I don't like how UserInput has a head and map. Ideally it should have some internal state indicating the last direction pushed and one getter (getDirection) that is used by the game loop (more on that below). It should definitely not have all of the logic of moving the snake! That should be the responsibility of Snake.
  • Why SNAKE_SPEED in Driver? What units is it in?
  • .getUser().getHead() is everywhere in your code. That should be a sign that some refactoring is necessary.

Now let's talk about the big idea. What you need is a game loop. No timers, no threads. You want the game loop pattern. The high level idea is that your game is a object with two methods update(double elapsedTime) and draw(). The first is called frequently and handles keyboard input, updating game state, checking collisions, determining win/loss scenarios. The second is called less frequently (but ideally at 60fps) after the game state is updated and is responsible for drawing the current game state to the screen.

More on that later. Let's start by simplifying your "game objects." We'll start by making an immutable Point. Why immutable? Well, simply put when things change it's very hard to keep all of that in the back of your mind while you're coding. Say for example that we created a Point with members public int x, y; Then Apple has a public Point position;. Anyone who has that apple can change its position. An apple should never move once placed. We have no way of ensuring that other than to keep in the back of our mind that we should never modify apple.position.x or y. But if we make Point immutable we can expose a Point getPosition() on Apple without worrying that objects/methods who we give that apple to will be able to move it around. Hopefully this convinces you.

Let's start with Point (Direction can remain the same):

class Point {
    protected int x;
    protected int y;

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

    public int getX() { return this.x; }
    public int getY() { return this.y; }

    public Point movedTowards(Direction d) {
        switch(d) {
            case Direction.UP:    return new Point(this.x - 1, this.y    );
            case Direction.DOWN:  return new Point(this.x + 1, this.y    );
            case Direction.LEFT:  return new Point(this.x    , this.y - 1);
            case Direction.RIGHT: return new Point(this.x    , this.y + 1);
        }
    }

    public boolean inBounds(int width, int height) {
        return 0 <= this.x && this.x < width && 0 <= this.y && this.y < height;
    }

    @Override
    public boolean equals(Object other) {
        if (this == other) {
            return true;
        }

        if (!(other instanceof Point)) {
            return false;
        }

        Point otherPoint = (Point) other;
        return this.x == otherPoint.x && this.y == otherPoint.y;
    }

    @Override
    public int hashCode() {
        return 31 * this.x + y;
    }
}

Good, now we have a nice encapsulated idea that can be used in the game objects that need it (Snake and Apple). We could also consider adding a static method to Point called randomPoint(int width, int height) that returns a random point between (0, 0) and (width, height). We'd have to be careful here, though (you make this mistake in your code as far as I can tell). We probably don't want to spawn an apple inside the snake's body/head. We'll tackle that later.

With this building block, let's build the Apple.

import javafx.scene.image.Image;

class Apple {
    protected Point position;
    protected static final Image IMAGE;

    static {
        IMAGE = new Image(new FileInputStream("resources/redsquare.jpg"));
    }

    public Apple(Point position) {
        this.position = position;
    }

    public Point getPosition() { return this.position; }

    public void draw(GridGraphicsContext ctx) {
        ctx.drawImage(IMAGE, position);
    }
}

Note a few things here. First, the apple's location is immutable. We want this! Also note how the apple is responsible for drawing itself (and loading its resources--although it is perhaps a little sloppy to do this in a static initializer; a more principled approach would be creating an AppleFactor which loads the file and builds apples by adding a paramter to the constructor for the Image, but this is a lot of Java-y indirection and this approach is sufficient for your simple use case). Also note how I've gotten rid of your GridView. You should be using Canvas. This is what real games do and the GridView was just an unnecessary level of indirection (and a lot of overhead of making arrays of ImageViews). I'd recommend making a GridGraphicsContext to wrap your GraphicsContext (from Canvas.getGraphicsContext()), to avoid duplicating the logic of computing the point for the image (converting integer grid coordinates to floating point canvas coordinates):

import javafx.scene.canvas.GraphicsContext;
import javafx.scene.image.Image;

class GridGraphicsContext {
    protected GraphicsContext ctx;

    public GridGraphicsContext(GraphicsContext ctx) {
        this.ctx = ctx;
    }

    public drawGridImage(Image image, Position p) {
        this.ctx.drawImage(image, p.getX() * image.getWidth(),
                           p.getY() * image.getHeight());
    }
}

Now let's look at Snake. Since we defined equals and hashCode on Point, it is hashable (we couldn't make it hashable if it was mutable!). This will let us represent the body of the snake a lot more succinctly.

import java.util.Deque;
import java.util.Set;
import java.util.ArrayDeque;
import java.util.HashSet;

class Snake {
    protected static final Image BODY_IMAGE;

    static {
        BODY_IMAGE = new Image(new FileInputStream("resources/blacksquare.png"));
    }

    protected Point headPosition;
    protected Deque<Point> bodyPositions = new ArrayDeque<>();
    protected Set<Point> bodyPositionsSet = new HashSet<>();

    public Snake(Point headPosition) {
        this.headPosition = headPosition;
    }

    public void move(Direction direction) {
        try {
            Point lastPosition = this.bodyPositions.removeLast();
            this.bodyPositionSet.remove(lastPosition);
        } catch (NoSuchElementException e) {}

        this.bodyPositions.offerFirst(this.headPosition);
        this.bodyPositionsSet.add(this.headPosition);

        this.headPosition = this.headPosition.moveTowards(direction);
    }

    public boolean intersectsSelf() {
        return this.headPositionsSet.contains(this.headPosition);
    }

    public boolean headIntersects(Apple apple) {
        return this.headPosition.equals(apple.getPosition());
    }

    public void draw(GridGraphicsContext ctx) {
        ctx.drawImage(BODY_IMAGE, this.headPosition);

        for (Point p : this.bodyPositions) {
            ctx.drawImage(BODY_IMAGE, p);
        }
    }
}

Notice how I use the interfaces for the types of the collections. This is a good habit to get into. Generally your types should be interfaces so that you can accept any kind of object that conforms to that interface. This allows you to swap out implementations by only changing one line of code. Also note how I changed how you represent the snake's head and body. For one, I kept the separate head member variable. There are multiple reasons for this. Although it could still be nulled, this conveys the idea that the snake always has a head, but may not have a body (bodyPositions and bodyPositionsSet may be empty). This is a good pattern because it's possible that bodyPositions.peekFirst() may throw if it is empty. Although we could get a NullPointerException if headPosition was null (there are static tools to prevent this), this conveys that idea and is a bit safer. Also note that with bodyPositions and bodyPositionsSet testing for collisions is now constant time and moving the snake is easier. Collision testing should be obvious. For moving the snake, note how that you don't need to keep track of the direction of each block. Instead, you can just remove the last body point, add the current head, and then move the head. This is a lot simpler to wrap your head around and ends up being much less work.

Now let's fix the user input:

import javafx.event.EventHandler;
import javafx.scene.input.KeyEvent;
import static javafx.scene.input.KeyCode.*;

class InputManager implements EventHandler<KeyEvent> {
    protected Direction direction = Direction.UP;

    public Direction getDirection() { return this.direction; }

    @Override
    public void handle(KeyEvent event) {
        if (event.getCode() == UP) {
            this.direction = Direction.UP;
        } else if (event.getCode() == DOWN) {
            this.direction = Direction.DOWN;
        } else if (event.getCode() == LEFT) {
            this.direction = Direction.LEFT;
        } else if (event.getCode() == RIGHT) {
            this.direction = Direction.RIGHT;
        }
    }
}

Note how much simpler this is! It has one concern: keeping track of which direction the user pressed. And a user of this object can ask what that direction was. And we can use that and bring everything together in the new Game object:

import javafx.application.Application;
import javafx.stage.Stage;
import javafx.scene.canvas.Canvas;
import javafx.scene.canvas.GraphicsContext;
import javafx.scene.Scene;

class Game extends Application {
    protected static final long TICK_TIME = 100; // miliseconds

    protected static final int COLS = 21;
    protected static final int ROWS = 21;

    protected static final int GRID_SIZE = 50; // size of tile image
    protected static final int WIDTH = COLS * GRID_SIZE;
    protected static final int HEIGHT = ROWS * GRID_SIZE;

    protected InputHandler inputHandler = new InputHandler();
    protected GraphicsContext graphicsContext;
    protected GridGraphicsContext gridGraphicsContext;

    protected Snake snake;
    protected Apple apple; 

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

    @Override
    public void start(Stage window) {
        window.setTitle("Snake");

        // It's been a long time since I've used javafx, so this might
        // not be totally correct. I believe you need a Parent to insert the
        // Canvas into (that becomes the root of the Scene). You can sort this out.
        Canvas canvas = new Canvas(WIDTH, HEIGHT);
        this.graphicsContext = canvas.getGraphicsContext();
        this.gridGraphicsContext = new GridGraphicsContext(this.graphicsContext);

        Scene scene = new Scene(canvas);
        scene.setOnKeyPressed(this.inputHandler);

        window.setScene(scene);
        window.show();

        this.startGameLoop();
    }

    public void startGameLoop() {
        long lastFrameTime = System.currentTimeMillis();

        while (true) {
            long currentTime = System.currentTimeMillis();
            long elapsedTime = currentTime - lastFrameTime;
            lastFrameTime = currentTime;

            if (!this.update(elapsedTime)) {
                break;
            }

            this.draw();
        }
    }

    protected long totalTime = 0;

    public boolean update(long elapsedTime) {
        this.totalTime += elapsedTime;

        while (this.totalTime > TICK_TIME) {
            this.totalTime -= TICK_TIME;

            if (this.snake.intersectsSelf() || !this.snake.getHeadPosition().inBounds(COLS, ROWS)) {
                this.reportLoss();
                return false;
            }

            this.snake.move(this.inputHandler.getDirection());

            if (this.snake.intersects(this.apple.getPosition()) {
                // TODO: increment score

                // TODO: don't intersect snake head or body
                this.apple = Apple.randomlyPositioned(COLS, ROWS);
            }
        }

        return true;
    }

    public void draw() {
        // Clear previous drawing (in a real game, this would be double buffered)
        this.graphicsContext.setFill(Color.WHITE);
        this.graphicsContext.fillRect(0, 0, WIDTH, HEIGHT);

        this.snake.draw(this.gridGraphicsContext);

        if (this.apple != null) {
            this.apple.draw(this.gridGraphicsContext);
        }
    }
}

This is a bit bulky, but you'll find that overall it's easier to follow. We still do the usual javafx setup (as I noted in the comment, I don't think I did it exactly correctly. You can sort of adding the Canvas to the stage). You may actually want to pull that out into your own Application class and then have Application own a Game object (which is only given the things it needs). But javafx aside, the only other thing we have is the game loop (startGameLoop()). This is a simple game loop (read the linked stuff for more complex ones that do fixed timestep increments and other fancy techniques used in real games--you don't need these though for such a simple game). We see it is just a while loop that only exits if update returns false (indicate the game is over). It alternates between calling update() and draw(). The idea here being we update state and then redraw the screen to show the player what changed. update receives the number of milliseconds that have elapsed since it was last called. This is usually more relevant for games with physics (where computing the next game state involves doing physics calculations--typically integrating some function given the elapsed time), but it works here too. We have a TICK_TIME which is the amount of milliseconds that elapses between each time the snake actually moves. We just keep track of how much time has elapsed and when more than a tick has elapsed, we actually preform the tick (do the game update). Note how we don't just do if (this.totalTime > TICK_TIME) and instead use a while. This is because it is possible that between updates more than 1 tick has happened (consider if the computer is under high load and the OS scheduler doesn't schedule our game for more than 200ms).

update() is otherwise fairly simple thanks to some of the separation of concerns that we undertook earlier. Checking for game end states is delegated to the proper objects. The same goes for eating apples.

draw() is even easier. It just defers to the appropriate drawing methods on the game objects. Now, since the game has tick based graphics, we could be a bit smarter about drawing and only redraw when something has changed. You could make this optimization, but if you wanted to, say, have a cool death animation you'd want to keep this structure.

I didn't try running this code, so I suspect there may be some issues (and I've indicated places where you need to implement things). But, this is the overall idea. Hopefully this can help you improve your game!

\$\endgroup\$

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.