Snake in JavaFX

I was unable to sleep on the 17th of December (after 12 AM IST) so I decided to make use of my time. I made a simple Snake game in JavaFX in 3 hours (roughly from 1 AM to 4 AM). Here are all the classes.

Organisation

src/astrobleme
 |
 +-- gui
 |    |
 |    +-- Main.java
 |    +-- Painter.java
 |
 +-- logic
      |
      +-- Food.java
      +-- GameLoop.java
      +-- Grid.java
      +-- Point.java
      +-- Snake.java

Food.java

package astrobleme.logic;

import javafx.scene.paint.Color;

/**
 * A simple class to represent food that takes up only one square.
 *
 * @author Subhomoy Haldar
 * @version 2016.12.17
 */
public class Food {
    public static final Color COLOR = Color.ROSYBROWN;

    private Point point;

    Food(Point point) {
        this.point = point;
    }

    public Point getPoint() {
        return point;
    }

    public void setPoint(Point point) {
        this.point = point;
    }
}

GameLoop.java

```
package astrobleme.logic;

import astrobleme.gui.Painter;
import javafx.scene.canvas.GraphicsContext;

/**
* @author Subhomoy Haldar
* @version 2016.12.17
*/
public class GameLoop implements Runnable {
private final Grid grid;
private final GraphicsContext context;
private int frameRate;
private float interval;
private boolean running;
private boolean paused;
private boolean keyIsPressed;

public GameLoop(final Grid grid, final GraphicsContext context) {
this.grid = grid;
this.context = context;
frameRate = 20;
interval = 1000.0f / frameRate; // 1000 ms in a second
running = true;
paused = false;
keyIsPressed = false;
}

@Override
public void run() {
while (running && !paused) {
// Time the update and paint calls
float time = System.currentTimeMillis();

keyIsPressed = false;
grid.update();
Painter.paint(grid, context);

if (!grid.getSnake().i

A few recommendations per class:

Food.java

What's the point in this class? It just's just a Point with a constant colour. I would just have a FOOD_COLOUR constant in your Painter and then you can scrap this.

GameLoop.java

frameRate and interval should be final. Actually you don't need frameRate at all because you don't use it for anything other than the calculation of interval. Additionally, your setFrameRate(int frameRate) function is broken because it doesn't update the interval. You don't call it anyway, so why not just delete it? Same with the getter.

Grid.java

I don't like that your constructor is creating Food and Snake objects. You should be passing these in. Annoying to ensure that they are positioned within the grid, though - you would probably have to throw an exception.

Grid is being used too much as a "dumb" bag of data. It has too many getters. Try to encapsulate as much as possible. getCols and getRows are again unused so you should just get rid of them. Rather than expose all your properties via getWidth, getHeight, getSnake and getFood, why not have a method which will paint a grid to a GraphicsContext:

public class Grid
{
   ...
   void paint(GraphicsContext gc)
   {
       gc.setFill(Grid.COLOR);
       gc.fillRect(0, 0, rows * SIZE, cols * SIZE);
       //etc...
   }
}

Point.java

A solid, immutable Point class which does everything you need it to. Very good.

Snake.java

If your Point deserved a class, xVelocity and yVelocity should absolutely be a class as well. If you renamed Point to represent a 2D Euclidean vector - Vec2D etc. - you could use the same class for both position and velocity without causing confusion.

Why are you bitwise and-ing here?

safe &= !points.contains(point);

In a bitwise and you always evaluate both sides, so you are always doing points.contains(point) even if you don't need to.

Painter.java

If you ever find yourself writing...

// Now the Food
...
// Now the snake
...
// The score

... then you should create a separate function. It's just as obvious what's going on, but more testable and maintainable.

paintFood(...)
{
   gc.setFill(Food.COLOR);
   paintPoint(grid.getFood().getPoint(), gc);
}

paintSnake(...)
{
   //whatever
}

Main.java

I would refactor your lambda to a separate class. It's too large.

canvas.setOnKeyPressed(e -> {

would become

setOnKeyPressed(new KeyHandler());
...
public class KeyHandler implements EventHandler
{
    void handle(KeyEvent event)
    {
         //do stuff
    }
}

If you're looking to change the key bindings in the future, where is it more likely you'll look first: Main.java or KeyHandler.java ?

Your start(Stage primaryStage) function suffers from the same "sectioning" as your Painter, except without the comments. Move the sections you have grouped together with whitespace to separate functions that better explain what they do.

Overall, I was quite impressed. It was very easy to follow and everything was named sensibly.

StackExchange Code Review Q#151800, answer score: 7