java - Basic snake game

Question

This is a snake game I made,

Note: at this point, I would like to hear any thoughts/ reviews about it.

Thank you

The game

Game class:

package snake;
import java.awt.Canvas;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.Graphics;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
import java.awt.image.BufferStrategy;
import javax.swing.JFrame;
public class Game extends Canvas implements Runnable{
 public static final int WIDTH = 720;
 public static final int HEIGHT = 720; 
 public static final int BLOCK_SIZE = 30; //Do not change - size of the food and snake body part
 //as well as their images
 private Thread thread;
 private boolean running;
 private Snake snake;
 private Food food;
 public Game(){
 initializeWindow();
 snake = new Snake(this);
 food = new Food();
 food.generateLocation(snake.getCopyOfEmptySpaces());
 initializeKeyAdapter();
 start();
 }
 private synchronized void start() {
 thread = new Thread(this);
 running = true;
 thread.start();
 this.requestFocus();
 }
 public void run() {
 double amountOfTicks = 10d; //ticks amount per second
 double nsBetweenTicks = 1000000000 / amountOfTicks;
 double delta = 0;
 long lastTime = System.nanoTime();
 while(running) {
 long now = System.nanoTime();
 delta += (now - lastTime) / nsBetweenTicks;
 lastTime = now;
 while (delta >= 1) {
 tick();
 delta--;
 }
 render();
 }
 }
 public void tick() {
 if (snake.isDead()) {
 running = false;
 }
 else {
 if (isEating()) {
 food.generateLocation(snake.getCopyOfEmptySpaces());
 }
 snake.tick();
 }
 }
 public void render() {
 if (running) {
 BufferStrategy bs = this.getBufferStrategy();
 if (bs == null) {
 this.createBufferStrategy(3);
 return;
 }
 Graphics g = bs.getDrawGraphics();
 g.setColor(Color.black);
 g.fillRect(0, 0, Game.WIDTH, Game.HEIGHT);
 food.render(g);
 snake.render(g);
 if (snake.isDead()) {
 g.setColor(Color.white);
 g.setFont(new Font("Tahoma", Font.BOLD, 75));
 g.drawString("Game Over", Game.WIDTH / 2 - 200 , Game.HEIGHT / 2);
 }
 g.dispose();
 bs.show();
 }
 }
 public boolean isEating() {
 return snake.getHeadCoor().equals(food.getCoor());
 }
 private JFrame initializeWindow() {
 JFrame frame = new JFrame("Snake Game");
 frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 frame.add(this);
 this.setPreferredSize(new Dimension(Game.WIDTH, Game.HEIGHT));
 frame.pack();
 frame.setLocationRelativeTo(null);
 frame.setVisible(true);
 return frame;
 }
 private void initializeKeyAdapter() { 
 //this is how to game gets keyboard input 
 //the controls are wasd keys
 class MyKeyAdapter extends KeyAdapter{
 private int velocity = Snake.DEFAULT_SPEED; //move a whole block at a time
 @Override
 public void keyPressed(KeyEvent e) {
 int key = e.getKeyCode();
 if (key == KeyEvent.VK_ESCAPE) {
 System.exit(0);
 }
 //after a key has been pressed we check if the snake goes the opposite way
 //if so, we ignore the press
 if (key == KeyEvent.VK_S) {
 if (snake.getVelY() != -velocity) {
 snake.setVel(0, velocity);
 }
 }
 else if (key == KeyEvent.VK_W) {
 if (snake.getVelY() != velocity) {
 snake.setVel(0, -velocity);
 }
 }
 else if (key == KeyEvent.VK_D) {
 if (snake.getVelX() != -velocity) {
 snake.setVel(velocity, 0);
 }
 }
 else if (key == KeyEvent.VK_A) {
 if (snake.getVelX() != velocity) {
 snake.setVel(-velocity, 0);
 }
 }
 }
 }
 this.addKeyListener(new MyKeyAdapter()); //adding it to the game
 }
 public static void main(String[] args) {
 Game g = new Game();
 }
}

Snake class:

package snake;
import java.awt.Graphics;
import java.awt.Image;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.Set;
import javax.swing.ImageIcon;
public class Snake {
 public static final int DEFAULT_SPEED = Game.BLOCK_SIZE;
 private Game game;
 private int velX;
 private int velY;
 private LinkedList<Coor> body; //snake's body
 private Set<Coor> emptySpaces; //valid spots for food- spots without snake parts
 private boolean dead;
 private Image img; //img of other body parts
 /*
 * @pre: Game.HEIGHT / Game.BLOCK_SIZE == 0 && Game.WIDTH / Game.BLOCK_SIZE == 0
 * @pre: Game.HEIGHT % 2 == 0
 * @pre: Game.WIDTH > 3 * Game.BLOCK_SIZE
 * @post: the snake starts at the middle of the screen
 */
 Snake(Game game){
 this.game = game;
 body = new LinkedList<Coor>();
 //starting snake
 int halfScreenHeight = Game.HEIGHT / 2;
 body.add(new Coor(2 * Game.BLOCK_SIZE, halfScreenHeight)); //head block
 body.add(new Coor(Game.BLOCK_SIZE, halfScreenHeight)); //middle block
 body.add(new Coor(0, halfScreenHeight)); //last block
 velX = DEFAULT_SPEED;
 initializeEmptySpaces();
 initializeImage();
 }
 public void tick() { //updating the body and checking for death
 /* Updating body:
 * Explanation: the Coor of the n-th body part is the Coor of the head n ticks ago
 * Execution: adding the current head Coor to the body, and pushing all other
 * Coors one place. If the snake hasn't eat this turn than we will remove 
 * the last Coor in the body. Oterwise, it has eat and needs to grow,
 * in that case we'll keep it
 * Result: the body will be: [Coor now, before 1 tick, before 2 ticks, ...] 
 */
 int prevHeadX = body.getFirst().getX();
 int prevHeadY = body.getFirst().getY();
 body.push(new Coor(prevHeadX + velX, prevHeadY + velY)); //new head Coor
 if (!game.isEating()) {
 Coor lastCoor = body.getLast();
 body.removeLast();
 emptySpaces.add(lastCoor); //now there is no body part on it 
 }
 emptySpaces.remove(getHeadCoor()); 
 checkDeath();
 }
 public void render(Graphics g) {
 for (Coor curr : body) {
 g.drawImage(img, curr.getX(), curr.getY(), null);
 }
 }
 private void checkDeath() {
 Coor h = getHeadCoor();
 if (h.getX() < 0 || h.getX() > Game.WIDTH - Game.BLOCK_SIZE) { //invalid X
 dead = true;
 }
 else if (h.getY() < 0 || h.getY() > Game.HEIGHT - Game.BLOCK_SIZE) { //invalid Y
 dead = true;
 }
 else {
 dead = false; 
 for (int i = 1; i < body.size(); i++) { //compare every non-head body part's coor with head's corr
 if (getHeadCoor().equals(body.get(i))) { //head touched a body part
 dead = true;
 }
 }
 }
 }
 public void setVel(int velX, int velY) {
 this.velX = velX;
 this.velY = velY;
 }
 public int getVelX() {
 return velX;
 }
 public int getVelY() {
 return velY;
 }
 public boolean isDead() {
 return dead;
 }
 public Set<Coor> getCopyOfEmptySpaces() {
 return new HashSet<Coor>(emptySpaces);
 }
 private void initializeEmptySpaces() {
 emptySpaces = new HashSet<Coor>();
 for (int i = 0; i * Game.BLOCK_SIZE < Game.WIDTH; i++) {
 for (int j = 0; j * Game.BLOCK_SIZE < Game.HEIGHT; j++) {
 emptySpaces.add(new Coor(i * Game.BLOCK_SIZE, j * Game.BLOCK_SIZE));
 }
 }
 emptySpaces.removeAll(body); //remove the starting snake parts
 }
 private void initializeImage() {
 ImageIcon icon = new ImageIcon("src/res/snake.png");
 img = icon.getImage();
 }
 public Coor getHeadCoor() {
 return body.getFirst();
 }
}

Food class:

package snake;
import java.awt.Graphics;
import java.awt.Image;
import java.util.Iterator;
import java.util.Random;
import java.util.Set;
import javax.swing.ImageIcon;
public class Food {
 private Image img;
 private Coor coor;
 Food(){
 initializeImages();
 }
 public void render(Graphics g) {
 g.drawImage(img, coor.getX(), coor.getY(), null);
 }
 public void generateLocation(Set<Coor> set) { //picking a random coordinate for the food
 int size = set.size();
 Random rnd = new Random();
 int rndPick = rnd.nextInt(size);
 Iterator<Coor> iter = set.iterator();
 for (int i = 0; i < rndPick; i++) {
 iter.next();
 }
 Coor chosenCoor = iter.next();
 coor = chosenCoor;
 }
 private void initializeImages() {
 ImageIcon icon = new ImageIcon("src/res/food.png");
 img = icon.getImage();
 }
 public Coor getCoor() {
 return coor;
 }
}

Coor class:

package snake;
public class Coor { //coordinates
 //we divide the screen to rows and columns, distance
 //between two rows or two columns is Game.BLOCK_SIZE
 private int x;
 private int y;
 Coor(int x, int y){
 this.x = x;
 this.y = y;
 }
 public int getX() { 
 return x;
 }
 public int getY() {
 return y;
 }
 @Override
 public String toString() {
 return "(" + x + ", " + y + ")";
 }
 @Override
 public int hashCode() {
 return x * Game.WIDTH + y;
 }
 @Override
 public boolean equals(Object o) {
 Coor c = (Coor) o;
 if (x == c.getX() && y == c.getY()) {
 return true;
 }
 return false;
 }
}

• dariosicily: Hello, have you checked Point class for coordinates?
• benjamin: I have, didn't want to cast to int every time I use getx() and getY() methods.
• dariosicily: Ok one question, you are overriding equals and hashCode methods in the Coor class. normally instanceof is present in equals method, have the system (ide, etc.) generated them ?
• benjamin: @dariosicily Actually I don't know (I think yes).. maybe someone else will answer it.

Answer 1

Coor has the comment coordinates ... yeah, that's exactly what the name should be then. But actually, Point seems easier and doesn't have to be abbreviated, or perhaps be more general and say Vector, or Vec2, that seems fairly common for games (despite it being an abbreviation). Not using the AWT class makes sense to me too.

The hashCode method is okay, though it could probably be a bit more random in its output (not that it matters for such small numbers of it.

The equals method could be more safe and also consider passing in arbitrary objects (or null) for comparison. Violating this is probably okay for this limited scope, but in general that shouldn't be skipped.

Also the return statement can be simplified.

@Override
public boolean equals(Object o) {
 if (o == null || !(o instanceof Coor)) {
 return false;
 }
 Coor c = (Coor) o;
 return x == c.getX() && y == c.getY();
}

---

The Food class uses these abbreviated names, img, rnd, etc. I'd suggest writing them out and giving them some more descriptive names in general.

The loop in generateLocation seems a bit bogus to me, why skip a random number of random numbers before picking one? If you have problems getting repeated numbers each run of the program you should perhaps initialise it from a truly random source.

---

Snake has velX and velY - that's exactly where a Vector would come in handy again. After all it's exactly that, a 2-tuple exactly like what Coor is.

checkDeath could use a for (x : body) ... for the death check, plus, once dead = true was set, a break would also be good.

---

Okay, so generally, I'd suggest not carrying around a set of empty spaces. Keeping the taken coordinates for the snake and for the food is fine. Using those you can immediately see which coordinates are empty ... all the ones that aren't taken. Given the few food items and the length of the snake the list of coordinates that's easy enough to check against.

Apart from that MyKeyAdapter (well that should be MyKeyAdaptor) is a bit weird how it's just inline there like that. And that goes for the other classes too, it's all mixing the representation via Swing with the game state and that's, at least for bigger games/projects, not advisable. Then again, it's snake. Just consider how you'd handle extending this code to encompass more features, like different kinds of objects, or how e.g. customisable key bindings would work.

So, it'd perhaps make sense to have a Renderable interface for the render method, then keep a list of objects to render in a more generic fashion, or even combine it with the tick method (perhaps with a default implementation on the interface) to update all game objects.

• benjamin: About the name abbreviation- wouldn't it make the code just longer? I guess it's true about coor since it's a class, but aren't img (image) and rnd (random) common abbreviations?
• benjamin: [I wanted to pm this one but apparently it's impossible] About the hashCode- I thought its role is to sort the same objects together (if easily done). since there is fairly easy way of doing so in that case I didn't want to use randoms. About generateLocation- can you explain "you should perhaps initialise it from a truly random source"? Since the hashCode isn't random the coordinates in the set will be in same order, so I wanted to skip some (random amount) of coors. *Would you use the same type for both coors and (velX, velY)?
• benjamin: *I didn't use foreach loop in checkDeath because I wanted to skip the head. *Doesn't using break is a bad practice? I thought you shuld use booleans for that. *(Last one) Should have I place the keyAdeptor in its own class even though it only used in the game? *** Thank you very much for the review. I totally agree on the before-last paragraph- is there a game-dev guide you recommend? Also if you could answer even some of my followup questions it would be great. btw sorry for my English :)
• ferada: Re abbreviation: Yes, there's different opinions on it. IMO, img for image isn't saving me much reading, as is rnd for random. One could argue single letter variables are even better then and indeed lots of Go or Haskell code uses single letter variables for better or worse. If you decide to keep it, just apply it consistently.
• ferada: hashCode is for ... hashing, if all objects had the same hashCode value they'd still need to be sortable according to equals. "... the coordinates in the set will be in the same order": okay, misunderstood the intent; well if the set is small (as it is here) this is the right approach. However, you'll obviously run into problems if your set is getting bigger and bigger. And lastly, yes, same data type for coordinates and velocity, both can just be (2-element) vectors (of int).