A simple Snakes and Ladders implementation in Java

Question

I created a pretty straight forward snakes and ladders implementation in Java just for the heck of it. The program can be summed up as-

1. Object of Board contains an array of Type Tile to represent the actual board.

2. Object of type Tile contains an Enum data member to identify if its a regular tile or a snake/ ladder.

   Also contains a destination tile to identify the snake/ ladder destination.

3. Player object contains the player name and player position.

4. On initialization, through randoms, I generate random tiles to be snakes and ladders. Then the user is asked to enter no. of players, and then keep rolling the dice till a player reaches the 100th tile.

The code:

Class BoardImpl:

import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

public class BoardImpl implements Board{

    Tile[] board = new Tile[101];         //A array to represent the 'board'
    List<Player> players = new ArrayList<>();
    SecureRandom secureRandom = new SecureRandom();
    Set<Integer> occupiedPositions = new HashSet<>();

    public void setPlayers(List<Player> players) {
        this.players = players;
    }

    public Tile[] getBoard() {
        return board;
    }

    public List<Player> getPlayers() {
        return players;
    }

    public void addPlayers(Player player) {
        this.players.add(player);
    }

    @Override
    public void initialize() {
//this function generates the board, placing random snakes and ladders and
//filling the rest with normal tiles
        System.out.println("Initializing board");

        int snakes = secureRandom.nextInt(15) + 1; //generates snakes & 
        int ladders = secureRandom.nextInt(15) + 1; //ladders at random
                                                    //positions
        setSnakes(snakes);
        setLadders(ladders);

        for(int i = 0; i < 101; i++){
            if(board[i] != null){
                continue;
            }
            Tile t  = new TileImpl(TileType.NORMAL);
            board[i] = t;
        }


    }

    @Override
    public void movePlayer(Player player, int places) {
        int currentPosition = player.getPosition();
        String checkPos = checkEndingPosition(currentPosition, places);
        if(checkPos.equals("winner")){
                winner(player);
        }
        if(checkPos.equals("outOfBoard")){
            System.out.println("That move cannot be made from your position! " +
                    "Roll a different number next time");
        }
        if(checkPos.equals("valid")){
                Tile t = getTile(currentPosition + places);
                if(t.getType().equals(TileType.NORMAL)){
                    player.setPosition(currentPosition + places);
                    System.out.println("Player " + player.getName() +
                            " moves to position "+ player.getPosition());
                }else {
                    player.setPosition(t.getDestination());
                    System.out.println(" Player " + player.getName() +
                            " encountered a "+ t.getType()+ "!!, moves to position "+ player.getPosition());
                }
        }
    }

    @Override
    public Tile getTile(int n) {
        return board[n];
    }

    public String checkEndingPosition(int currentPosition, int places){

        if((currentPosition + places) > 100){
            return "outOfBoard";
        }else if((currentPosition+ places) == 100){
            return "winner";
        }
        else{
            return "valid";
        }
    }

    public void winner(Player player){

        System.out.print("Player " + player.getName() + " has won the game !");
        System.exit(0);
    }

    public void setSnakes(int n){
//this function generates snakes at random positions
        for(int i = 0; i < n; i ++){
            boolean flag = true;
            int start = 0;
            int dest = 0;

            while(flag) {
                 start = secureRandom.nextInt(98);
                if(!occupiedPositions.contains(start)){
                    occupiedPositions.add(start);
                    break;
                }
            }

            while(flag) {
 //Setting the destination for a snake tile to lower than its position
                 dest = (int)(start * secureRandom.nextDouble());
                if(!occupiedPositions.contains(dest)){
                    occupiedPositions.add(dest);
                    break;
                }
            }

            Tile tile = new TileImpl(TileType.SNAKE, dest);
            board[start] = tile;

            System.out.println("Created snake " + "[" + start+ "," + dest + "]");
        }

    }

    public void setLadders(int n) {
//this function generates ladders randomly
        for (int i = 0; i < n; i++) {
            boolean flag = true;
            int start = 0;
            int dest = 0;

            while (flag) {
                start = secureRandom.nextInt(80);
                if (!occupiedPositions.contains(start)) {
                    occupiedPositions.add(start);
                    break;
                }
            }

            while (flag) {
//this step places the destination for the ladder tile to greater than its position
                dest = (int) (start + secureRandom.nextDouble() * 10);
                if (!occupiedPositions.contains(dest)) {
                    occupiedPositions.add(dest);
                    break;
                }
            }

            Tile tile = new TileImpl(TileType.LADDER, dest);
            board[start] = tile;
            System.out.println("Created ladder " + "[" + start+ "," + dest + "]");
        }
    }
}

Class PlayerImpl:

public class PlayerImpl implements Player{


    private int position = 0;
    private String name = null;

    public PlayerImpl(String name){
        this.name = name;
    }

    @Override
    public int getPosition() {
        return position;
    }

    @Override
    public String getName() {
        return name;
    }

    @Override
    public void setPosition(int currentPosition) {
        this.position = currentPosition;
    }

    @Override
    public void setName(String playerName) {
        this.name = playerName;
    }
}

Class TileImpl:

public class TileImpl implements Tile {

    private TileType type;
    private int position;
    private int destination;

    public TileImpl(TileType type) {
        this.type = type;
    }

    public TileImpl(TileType type, int dest) {
        this.type = type;
        this.destination = dest;
    }

    public int getPosition() {
        return position;
    }

    public int getDestination() {
        return destination;
    }

    public void setType(TileType type) {
        this.type = type;
    }

    public void setPosition(int position) {
        this.position = position;
    }

    public void setDestination(int destination) {
        this.destination = destination;
    }

    @Override
    public TileType getType() {
        return type;
    }

}

Class SnakesAndLadders:

import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class SnakesAndLadders {

    public static void main(String args[]){

        Board board = new BoardImpl();
        List<Player> players = new ArrayList<>();
        System.out.println("How many players ?");

        Scanner scan = new Scanner(System.in);
        int noOfPlayers = scan.nextInt();

        for(int i = 0; i < noOfPlayers; i++){
            System.out.println("Enter name for player "+ i);
            String name = scan.next();
            Player player = new PlayerImpl(name);
            players.add(player);

        }

        board.initialize();
        int counter = 0;
        System.out.println("Lets Play!");
        String choice = "";
        SecureRandom random = new SecureRandom();
//simulating a game through the do while loop
        do{
            if(counter >= noOfPlayers) counter = 0;
            Player currPlayer = players.get(counter);
            System.out.println(" Player " + currPlayer.getName() + " turn to play!");
            System.out.println(" R = Roll the dice, Q = quit");
            choice = scan.next();
            if(choice.equalsIgnoreCase("R")){
                int places = random.nextInt(6) + 1;
                board.movePlayer(currPlayer, places);
                counter++;

            }
        }while(!choice.equalsIgnoreCase("Q"));
    }
}

I think the game is very straight forward. I wanted to particularly know if

1. There is a better way to generate the random numbers of snakes and ladders.

2. A better and more appropriate data structure to represent the 'board'

3. If my code can be improved to contain any important paradigms/practices.

Answer 1

Welcome to Code Review. Nice implementation and good structure of the code, find my suggestions in line.

1. There is a better way to generate the random numbers of snakes and ladders.

The methods setSnakes and setLadders differ by just a couple of lines of code, which is an indicator of code duplication. This is the method setLadders:

public void setLadders(int n) {
//this function generates ladders randomly
    for (int i = 0; i < n; i++) {
        boolean flag = true;
        int start = 0;
        int dest = 0;

        while (flag) {
            start = secureRandom.nextInt(80);
            if (!occupiedPositions.contains(start)) {
                occupiedPositions.add(start);
                break;
            }
        }

        while (flag) {
//this step places the destination for the ladder tile to greater than its position
            dest = (int) (start + secureRandom.nextDouble() * 10);
            if (!occupiedPositions.contains(dest)) {
                occupiedPositions.add(dest);
                break;
            }
        }

        Tile tile = new TileImpl(TileType.LADDER, dest);
        board[start] = tile;
        System.out.println("Created ladder " + "[" + start+ "," + dest + "]");
    }
}

One approach is to create a method to generate the random numbers and then use it to generate ladders and snakes. For example:

private void setRandomLadder() {
    while (true) {
        int[] segment = getRandomSegment(1,99);
        int start = segment[0];
        int end = segment[1];
        if (isTileFree(start) && isTileFree(end)) {
            board[start] = new TileImpl(TileType.LADDER, end);
            board[end] = new TileImpl(TileType.OCCUPIED);
            break;
        }
    }
}

This method can be invoked n times depending on how many ladders you want to add to the board. The method getRandomSegment returns two random points between 1 and 99. Also I changed the name from setLadders to setRandomLadder, to make it self-descriptive. Finally, keep in mind that if you add too many ladders such methods never complete, so consider to check if there are free tiles before.

---

2. A better and more appropriate data structure to represent the 'board'

By using the previous method, we can remove the list occupiedPositions, which was possible by adding TileType.OCCUPIED.

The array of Tile is ok as data structure, an alternative is to use ArrayList.

---

3. If my code can be improved to contain any important paradigms/practices.

• Console output:

  How many players ?
  2
  Enter name for player 0
  

  It's confusing for the user to see player 0, start from player 1.

• Magic numbers: new Tile[101], secureRandom.nextInt(98), etc. Create constants, for example:

  private static final int SIZE = 101;
  

• Modifiers: the methods setSnakes and setLadders should be private, same for all the instance variables.

• Initialize in the constructor: the BoardImpl has a method initialize() but and empty constructor. You can use directly the constructor.

• Encapsulation: the interface Board accepts a Player with a public setPosition, which means anyone can change it. Only the board should change the state of the player. A safer interface might be:

  public interface Board {
    void movePlayer(String playerId, int places);
    //..
  }
  

• Avoid temporary variables: instead of

  Tile tile = new TileImpl(TileType.LADDER, dest);
  board[start] = tile;
  

  You can write:

  board[start] = new TileImpl(TileType.LADDER, dest);
  

• Code smell:

      public String checkEndingPosition(int currentPosition, int places){
  
        if((currentPosition + places) > 100){
            return "outOfBoard";
        }else if((currentPosition+ places) == 100){
            return "winner";
        }
        else{
            return "valid";
        }
    }
  

  Returning a String is very error prone here. For example, you could return a boolean and eventually an exception.

• Testing and reusability: the method winner calls System.exit(0), which makes it impossible to test. Additionally, there are many System.out.println in the BoardImpl class. It is very hard to test a class like that. Ideally, the class Board should never print to the user, only for debugging. The main should be responsible to interact with the user.

Answer 2

Readability and style

Please run your code through a formatter to make it more readable and follow standards.

This one may work:

https://www.tutorialspoint.com/online_java_formatter.htm

Flag?

            boolean flag = true;
            int start = 0;
            int dest = 0;

            while (flag) {
                start = secureRandom.nextInt(80);
                if (!occupiedPositions.contains(start)) {
                    occupiedPositions.add(start);
                    break;
                }
            }

You're not using, checking or changing the flag here. This loop should just be while(true) and the next loop does the same thing.

If-else-

if(checkPos.equals("winner")){
                winner(player);
        }

When you're checking several exclusive cases and comparing to a string, I think a switch statement is a more appropriate/standard way of doing it. It also makes it clearer that the cases are exclusive.

Data structure

Set<Integer> occupiedPositions = new HashSet<>();

This seems unnecessary/unnatural to me. I would add to the Tile class a bool to mark a tile as occupied, and then that information can be contained in the tile/board itself instead of having a separate Set for bookkeeping.