Una semplice implementazione di Snakes and Ladders in Java

Sep 22 2020

Ho creato un'implementazione piuttosto semplice di serpenti e scale in Java solo per il gusto di farlo. Il programma può essere riassunto come-

  1. Object of Board contiene un array di Type Tile per rappresentare il tabellone effettivo.

  2. L'oggetto di tipo Tile contiene un membro dati Enum per identificare se si tratta di una tessera normale o di un serpente / scala.

    Contiene anche una tessera di destinazione per identificare la destinazione del serpente / scala.

  3. L'oggetto giocatore contiene il nome e la posizione del giocatore.

  4. All'inizializzazione, tramite random, genero tessere casuali per essere serpenti e scale. Quindi all'utente viene chiesto di inserire no. di giocatori, quindi continua a tirare i dadi finché un giocatore non raggiunge la 100a tessera.

Il codice:

Classe 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 + "]");
        }
    }
}

Classe 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;
    }
}

Classe 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;
    }

}

Classe 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"));
    }
}

Penso che il gioco sia molto semplice. Volevo sapere in particolare se

  1. C'è un modo migliore per generare i numeri casuali di serpenti e scale.

  2. Una struttura dati migliore e più appropriata per rappresentare il 'consiglio'

  3. Se il mio codice può essere migliorato per contenere eventuali paradigmi / pratiche importanti.

Risposte

2 Marc Sep 23 2020 at 09:48

Benvenuto in Code Review. Bella implementazione e buona struttura del codice, trova i miei suggerimenti in linea.

  1. C'è un modo migliore per generare i numeri casuali di serpenti e scale.

I metodi setSnakese setLaddersdifferiscono solo per un paio di righe di codice, che è un indicatore della duplicazione del codice. Questo è il metodo 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 + "]");
    }
}

Un approccio consiste nel creare un metodo per generare i numeri casuali e quindi utilizzarlo per generare scale e serpenti. Per esempio:

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;
        }
    }
}

Questo metodo può essere richiamato più nvolte a seconda di quante scale vuoi aggiungere alla scheda. Il metodo getRandomSegmentrestituisce due punti casuali compresi tra 1 e 99. Inoltre ho cambiato il nome da setLaddersa setRandomLadder, per renderlo auto-descrittivo. Infine, tieni presente che se aggiungi troppe scale tali metodi non vengono mai completati, quindi considera di controllare se ci sono tessere libere prima.


  1. Una struttura dati migliore e più appropriata per rappresentare il 'consiglio'

Utilizzando il metodo precedente, possiamo rimuovere l'elenco occupiedPositions, che era possibile aggiungendo TileType.OCCUPIED.

L'array di Tileva bene come struttura dati, un'alternativa è usare ArrayList.


  1. Se il mio codice può essere migliorato per contenere eventuali paradigmi / pratiche importanti.
  • Uscita console :

    How many players ?
    2
    Enter name for player 0
    

    È fonte di confusione per l'utente vedere il giocatore 0, a partire dal giocatore 1.

  • Numeri magici : new Tile[101], secureRandom.nextInt(98), ecc Crea costanti, per esempio:

    private static final int SIZE = 101;
    
  • Modificatori : i metodi setSnakese setLaddersdovrebbero essere privati, lo stesso per tutte le variabili di istanza.

  • Inizializza nel costruttore : BoardImplha un metodo initialize()ma e un costruttore vuoto. Puoi usare direttamente il costruttore.

  • Incapsulamento : l'interfaccia Boardaccetta un Playercon un pubblico setPosition, il che significa che chiunque può cambiarlo. Solo il tabellone dovrebbe cambiare lo stato del giocatore. Un'interfaccia più sicura potrebbe essere:

    public interface Board {
      void movePlayer(String playerId, int places);
      //..
    }
    
  • Evita variabili temporanee : invece di

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

    Tu puoi scrivere:

    board[start] = new TileImpl(TileType.LADDER, dest);
    
  • Odore di codice :

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

    La restituzione di una stringa è molto soggetta a errori qui. Ad esempio, potresti restituire un booleano e alla fine un'eccezione.

  • Test e riusabilità : il metodo winnerchiama System.exit(0), il che rende impossibile il test. Inoltre, ce ne sono molti System.out.printlnnella BoardImplclasse. È molto difficile testare una classe del genere. Idealmente, la classe Boardnon dovrebbe mai stampare per l'utente, solo per il debug. Il maindovrebbe essere responsabile per interagire con l'utente.

3 user985366 Sep 22 2020 at 15:53

Leggibilità e stile

Esegui il codice tramite un formattatore per renderlo più leggibile e seguire gli standard.

Questo potrebbe funzionare:

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

Bandiera?

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

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

Non stai usando, controllando o cambiando il flag qui. Questo ciclo dovrebbe essere while(true)e il ciclo successivo fa la stessa cosa.

Se altro-

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

Quando controlli diversi casi esclusivi e li confronti con una stringa, penso che switchun'istruzione sia un modo più appropriato / standard per farlo. Rende anche più chiaro che i casi sono esclusivi.

Struttura dati

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

Questo mi sembra inutile / innaturale. Aggiungerei alla classe delle booltessere a per contrassegnare una tessera come occupata, e quindi quell'informazione può essere contenuta nella tessera / tabellone stesso invece di avere un separato Setper la contabilità.