6
\$\begingroup\$

I put my code below to ask you for a review and ask you if this can be considered a good and clean code?

The game works just fine, I have encountered one issue though - the Food condition to not to create new food on current snake position is not working, and to be honest with you - I have been trying to solve it, but forgot about it and now when I am posting this I have this in my mind one more time.

I have added some buttons to stop/pause the game (can be done by pressing Space as well) and choose speed. Score is related to speed value (score for food is the same as the number of speed).

What's the most interesting I guess is the fact, that almost all snake tutorials around the internet allow you (if you are fast enough) to bug the game with those simple steps: Assume, that the snake moves to the Right, timer is set to some amount of time, and if you manage to press Up or Down arrow and then very fast Left (when the timer hasn't gone off), the snake will move backwards and finally eat himself. Most of the games doesn't allow you to change from Right to Left (or Up to Down etc.) in one step, but this is allowed. I prevent players from doing that by implementing temporary direction, that reads the current direction at the begining of a time period (or refresh rate - it is the same here).

I would like to hear your comments, because I am not quite sure if I get (and if I can implement that) all the Object-oriented programming right. Thank you!

package com.RGuSnake;

public class Main {
    public static void main(String[] args) {

        Snake.getInstance().createBoard();

    }
}

package com.RGuSnake;

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Random;


public class Snake extends JPanel implements ActionListener {


    private int sizeWidth;
    private int sizeHeight;
    private int offsetWidth;
    private int offsetHeight;
    private int scale;
    private ArrayList<Point> snakeLocation;
    private static Point food;
    private String direction = "RIGHT";
    private String tmpDirection = "RIGHT";

    private static final Snake snake = new Snake();

    private Integer delay;
    private Boolean isPaused = false;
    private Boolean isAlive = false;
    private Timer timer;
    private Board board;
    private Buttons buttons;
    private JFrame frame;

    private Integer score=0;
    private int speed=5;

    private Snake() {

    }

    public static Snake getInstance() {
        return snake;
    }

    public void createBoard() {
        frame = new JFrame("Typical Snake Game");
        snakeLocation = new ArrayList<>();
        snakeLocation.add(new Point(-100,-100));
        food=new Point(-100,-100);
        board = new Board();
        sizeWidth=board.getSizeWidth();
        sizeHeight=board.getSizeHeight();
        offsetHeight=board.getOffsetHeight();
        offsetWidth=board.getOffsetWidth();
        scale=board.getScale();

        buttons = new Buttons();
        frame.getContentPane().add(BorderLayout.CENTER, board);
        frame.getContentPane().add(BorderLayout.SOUTH, buttons);
        frame.setPreferredSize(new Dimension(sizeWidth + 2 * offsetWidth, sizeHeight + 2 * offsetHeight + 50));

        frame.setResizable(false);
        frame.setVisible(true);
        frame.pack();
        frame.setLocationRelativeTo(null);
        frame.setFocusable(true);
        frame.requestFocus();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    }


    public void startGame() {
        delay=100+(5-speed)*15;
        System.out.println(delay);
        timer = new Timer(delay, this);
        System.out.println(delay);
        if(frame==null){
            snake.createBoard();
        }
        score=0;
        direction="RIGHT";
        snakeLocation.clear();

        for(int i=0;i<6;i++){
            snakeLocation.add(new Point(Math.round((sizeWidth+offsetWidth)/(2*10))*10-i*10, Math.round((sizeHeight+offsetHeight)/(2*10))*10));
        }

        newFood();

        buttons.blockButtons();
        isAlive = true;
        isPaused = false;
        timer.start();
    }



    public ArrayList<Point> getSnakeLocation() {
        return snakeLocation;
    }

    public Point getFoodLocation() {
        return food;
    }
    public Boolean getIsAlive() { return isAlive;}

    public void setDirection(String dir) {
        snake.direction = dir;
    }
    public String getDirection() {
        return snake.direction;
    }

    public String getTmpDirection(){
        return snake.tmpDirection;
    }

    public void spacePressed(){

        if(!isAlive) {
            snake.startGame();
        }
        else {
            isPaused^=true;
        }
    }

    public Boolean getPause(){
        return isPaused;
    }

    public void move() {
        if (direction.equals("RIGHT")) {
            snakeLocation.add(0, new Point(snakeLocation.get(0).x + 10, snakeLocation.get(0).y + 0));
        } else if (direction.equals("LEFT")) {
            snakeLocation.add(0, new Point(snakeLocation.get(0).x - 10, snakeLocation.get(0).y + 0));
        } else if (direction.equals("UP")) {
            snakeLocation.add(0, new Point(snakeLocation.get(0).x, snakeLocation.get(0).y - 10));
        } else if (direction.equals("DOWN")) {
            snakeLocation.add(0, new Point(snakeLocation.get(0).x, snakeLocation.get(0).y + 10));
        }
    }


    public void actionPerformed(ActionEvent arg0) {
        if(!isPaused && isAlive) {
            tmpDirection = direction;
            snake.move();

            snake.checkPosition();

            //refresh();
            board.repaint();

        }  else if(!isAlive) {
            timer.stop();
            buttons.enableButtons();
        }


    }

    public void newFood() {
        Random random = new Random();
        Point point;
        point = new Point(random.nextInt(sizeWidth / scale) * scale + offsetWidth, random.nextInt(sizeHeight / scale) * scale + offsetHeight);

        while (Arrays.asList(getSnakeLocation()).contains(point)) {
            point = new Point(random.nextInt(sizeWidth / scale) * scale + offsetWidth, random.nextInt(sizeHeight / scale) * scale + offsetHeight);
        }

        food = point;
    }

    public void increaseScore() {
       score=score+speed;
    }
    public int getScore(){
        return score;
    }

    public void increaseSpeed(){
        if(speed<10) {
            speed += 1;
        }
    }

    public void decreaseSpeed(){
        if(speed>1) {
            speed -= 1;
        }
    }

    public int getSpeed(){
        return speed;
    }

    public void refresh(){

        board.repaint();
    }


    public void checkPosition(){
        for (int j = 1; j < snakeLocation.size()-1; j++) {
            if (snakeLocation.get(0).equals(snakeLocation.get(j))) {
                isAlive = false;
            }
        }


            if (snakeLocation.get(0).x==offsetWidth-scale || snakeLocation.get(0).x==sizeWidth+offsetWidth ||snakeLocation.get(0).y==offsetHeight-scale || snakeLocation.get(0).y==sizeHeight+offsetHeight) {
                isAlive = false;
            }


        if (snakeLocation.get(0).equals(food)) {
            newFood();
            increaseScore();
        }
        else {
            snakeLocation.remove(snakeLocation.size() - 1);
        }

    }
}

package com.RGuSnake;

import javax.swing.*;
import java.awt.*;
import java.util.ArrayList;


public class Board extends JPanel {

    private int sizeWidth = 300;
    private int sizeHeight = 300;
    private int offsetWidth = 30;
    private int offsetHeight = 30;
    private int scale = 10;
    Snake snake=Snake.getInstance();

    public void paintComponent(Graphics g) {


            super.paintComponent(g);

            //super.update(g);
            ArrayList<Point> points = Snake.getInstance().getSnakeLocation();
            g.setColor(Color.BLACK);
            g.fillRect(offsetWidth - scale, offsetHeight - scale, sizeWidth + 2 * scale, sizeHeight + 2 * scale);
            g.setColor(Color.LIGHT_GRAY);
            g.fillRect(offsetWidth, offsetHeight, sizeWidth, sizeHeight);

            g.setColor(Color.BLACK);
            for (int i = 1; i < points.size(); i++) {
                g.fillRect(snake.getSnakeLocation().get(i).x, snake.getSnakeLocation().get(i).y, scale, scale);
            }
            g.setColor(Color.RED);
            g.fillRect(snake.getSnakeLocation().get(0).x, snake.getSnakeLocation().get(0).y, scale, scale);

            g.setColor(Color.BLUE);
            g.fillRect(snake.getFoodLocation().x, snake.getFoodLocation().y, scale, scale);

            g.setColor(Color.RED);
            Font font = new Font("Verdana", Font.BOLD, 12);
            g.setFont(font);
            FontMetrics fm = g.getFontMetrics();
            String score = "Score: " + snake.getScore() + "           Speed: " + snake.getSpeed();
            g.drawString(score, (offsetWidth * 2 + sizeWidth) / 2 - fm.stringWidth(score) / 2, offsetHeight / 2);


            if (!snake.getIsAlive()) {


                font = new Font("Verdana", Font.BOLD, 12);
                g.setFont(font);
                String gameOver1 = "CHOOSE THE SPEED";
                String gameOver2 = "BEFORE STARTING NEW GAME";
                fm = g.getFontMetrics();

                g.setColor(Color.red);
                g.drawString(gameOver1, (offsetWidth * 2 + sizeWidth) / 2 - fm.stringWidth(gameOver1) / 2, (offsetHeight + sizeHeight) / 2);
                g.drawString(gameOver2, (offsetWidth * 2 + sizeWidth) / 2 - fm.stringWidth(gameOver2) / 2, (offsetHeight + sizeHeight + 40) / 2);
                //String speed = "Game speed: " + snake.getSpeed();
                //g.drawString(speed,(offsetWidth*2+sizeWidth)/2-fm.stringWidth(speed)/2,(offsetHeight+sizeHeight)/2+70);
            }
        }


    public int getSizeWidth() {
        return sizeWidth;
    }

    public int getOffsetWidth() {
        return offsetWidth;
    }

    public int getSizeHeight(){
        return sizeHeight;
    }

    public int getOffsetHeight() {
        return offsetHeight;
    }

    public int getScale(){
        return scale;
    }    
}

package com.RGuSnake;

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;

public class Buttons extends JPanel {

    Snake snake=Snake.getInstance();
    Board board=new Board();
    JButton startGame;
    JButton speedDown;
    JButton speedUp;

    public Buttons(){
        JPanel buttonsPanel = new JPanel();
        buttonsPanel.setPreferredSize(new Dimension(board.getSizeWidth()+board.getOffsetWidth(), 20));

        startGame = new JButton("START!");
        startGame.setBackground(Color.red);

        speedDown = new JButton("Speed Down");
        speedUp = new JButton("Speed Up");
        startGame.addActionListener(new startGame());
        speedUp.addActionListener(new SpeedUp());
        speedDown.addActionListener(new SpeedDown());

        startGame.setFocusPainted(false);
        startGame.setFocusable(false);
        speedDown.setFocusPainted(false);
        speedDown.setFocusable(false);
        speedUp.setFocusPainted(false);
        speedUp.setFocusable(false);
        buttonsPanel.setLayout(new GridLayout());
        buttonsPanel.add(startGame);
        buttonsPanel.add(speedDown);
        buttonsPanel.add(speedUp);

        InputMap im = buttonsPanel.getInputMap(JPanel.WHEN_IN_FOCUSED_WINDOW);
        ActionMap am = buttonsPanel.getActionMap();

        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_RIGHT, 0), "RightArrow");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_LEFT, 0), "LeftArrow");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_UP, 0), "UpArrow");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_DOWN, 0), "DownArrow");
        im.put(KeyStroke.getKeyStroke(KeyEvent.VK_SPACE, 0), "Space");

        am.put("RightArrow", new ArrowAction("RightArrow"));
        am.put("LeftArrow", new ArrowAction("LeftArrow"));
        am.put("UpArrow", new ArrowAction("UpArrow"));
        am.put("DownArrow", new ArrowAction("DownArrow"));
        am.put("Space", new ArrowAction("Space"));
        add(buttonsPanel);

    }

    public void blockButtons() {
        startGame.setText("PAUSE");
        speedUp.setEnabled(false);
        speedDown.setEnabled(false);

    }

    public void enableButtons() {
        startGame.setText("START!");
        speedUp.setEnabled(true);
        speedDown.setEnabled(true);

    }

    private class startGame implements ActionListener {
        @Override
        public void actionPerformed(ActionEvent e) {
            snake.spacePressed();
        }
    }

    private class SpeedUp implements ActionListener {
        @Override
        public void actionPerformed(ActionEvent e) {
            snake.increaseSpeed();
            snake.refresh();
        }
    }
    private class SpeedDown implements ActionListener {
        @Override
        public void actionPerformed(ActionEvent e) {
            snake.decreaseSpeed();
            snake.refresh();
        }
    }
}

package com.RGuSnake;

import javax.swing.*;
import java.awt.event.ActionEvent;

public class ArrowAction extends AbstractAction {
    Snake snake=Snake.getInstance();
    private String cmd;

    public ArrowAction(String cmd) {
        this.cmd = cmd;
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        if ((cmd.equalsIgnoreCase("LeftArrow")) && !snake.getDirection().equals("RIGHT") && !snake.getTmpDirection().equals("RIGHT") && !snake.getPause()) {
            snake.setDirection("LEFT");
        } else if (cmd.equalsIgnoreCase("RightArrow") && !snake.getDirection().equals("LEFT") && !snake.getTmpDirection().equals("LEFT")&& !snake.getPause()) {
            snake.setDirection("RIGHT");
        } else if (cmd.equalsIgnoreCase("UpArrow")&& !snake.getDirection().equals("DOWN") && !snake.getTmpDirection().equals("DOWN")&& !snake.getPause()) {
            snake.setDirection("UP");
        } else if (cmd.equalsIgnoreCase("DownArrow")&& !snake.getDirection().equals("UP") && !snake.getTmpDirection().equals("UP")&& !snake.getPause()) {
            snake.setDirection("DOWN");
        } else if (cmd.equalsIgnoreCase("Space")) {
            snake.spacePressed();
        }
    }

}

enter image description here

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

I put my code below to ask you for a review and ask you if this can be considered a good and clean code?

Your program seems to suffer from the god object anti-pattern. In short: your Snake class does almost everything.

An easy starting point to mitigate this is saving the board instance as a field in the Snake class and move some board-related functionality to the Board class (the Composition pattern).

That way you can remove the following fields from your Snake class:

private int sizeWidth;
private int sizeHeight;
private int offsetWidth;
private int offsetHeight;
private int scale;

If we do need one of those variables you just use the getter on the board:

board.getWidth()

Next look at where those variables are used and see if that functionality could fit in the Board class instead.

For example the newFood() method. My first thought is to put this method inside the Board class. And from the Snake class (or whoever triggers spawning a new piece of food later on) just do board.spawnNewFood().

This also means we can move

private static Point food;

inside the board as well. The idea is to remove all food related methods from the Snake class. It shouldn't be too hard to finish this part yourself.

I suggest to also add a method to Board to return a random spawning location for a new snake. This way if you later add some new special squares to your board like walls, you can make sure to spawn the snake in a valid location.

A major change I suggest is to split up the Snake class into a class that represents the game (with all the buttons, and initiating game state and coupling the other classes) and another that actually represents a Snake (with a position and movement and other snake related stuff).

This change is a bit too big to post in my answer here though, so you'll have to try yourself first.

Now that I pointed out the first steps to improving the cleanliness of your code, I'll try to address the problem of where to move next.

Your tmpDirection was a good first attempt, but I suggest to take a slightly different approach.

The 10 in move() is a magic number, declare it as a field or a parameter to move() (recommended), as (a suggestion) double moveDistance. If you're sure that there will never be a need to change that value during the running of the program (think about it, if the user resizes the window, you should handle it gracefully and scale moveDistance to the playing area for the configured speed)

Since the direction can have only one among a specific set of values, an enum is recommended:

public enum Direction {
    UP, DOWN, LEFT, RIGHT
}

I suggest to also keep 2 variables for the movement in the snake:

private Direction currentDirection = Direction.RIGHT;
private Direction nextDirection = Direction.RIGHT;

The currentDirection is the direction the snake is currently moving. The nextDirection is the direction the snake will move on the next call to move().

Now in ArrowAction modify nextDirection instead of currentDirection. This means that if you quickly press and then , your ArrowAction for UP will change nextDirection to UP, and your ArrowAction for LEFT will not allow it, since the currentDirection is still going RIGHT.

Then also modify the move implementation slightly to first update the currentDirection:

// Assuming that `Direction` has been imported into scope
public void move() {
   currentDirection = nextDirection;

   switch(currentDirection) {
       case UP: snakeLocation.add(0, new Point(snakeLocation.get(0).x, snakeLocation.get(0).y - 10));
       break;
       case DOWN: snakeLocation.add(0, new Point(snakeLocation.get(0).x, snakeLocation.get(0).y + 10));
       break;
       case LEFT: snakeLocation.add(0, new Point(snakeLocation.get(0).x - 10, snakeLocation.get(0).y));
       break;
       case RIGHT: snakeLocation.add(0, new Point(snakeLocation.get(0).x + 10, snakeLocation.get(0).y));
       break;
       default: throw new IllegalArgumentException("Unknown move direction");
   }
}

A minor gameplay suggestion:

Increase the speed proportionally with the length of the snake (the amount of food consumed) at a higher difficulty level.

I hope these tips are enough to get the idea on how to improve your code. Good luck!

\$\endgroup\$
  • \$\begingroup\$ Thank you for this long post, I will try to implement those changes :) This kind of an answer is exactly the one I was expecting to hear! \$\endgroup\$ – user134131 Mar 23 '17 at 21:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy