Snakes and Ladders ใช้งานง่ายใน Java

Question

ฉันสร้างงูและบันไดที่ค่อนข้างตรงไปตรงมาใน Java เพื่อความสำเร็จ โปรแกรมสามารถสรุปได้เป็น -

1. Object of Board มีอาร์เรย์ของ Type Tile เพื่อแสดงถึงบอร์ดจริง

2. ออบเจ็กต์ประเภท Tile มีสมาชิกข้อมูล Enum เพื่อระบุว่าไทล์ปกติหรืองู / บันได

   นอกจากนี้ยังมีไทล์ปลายทางเพื่อระบุปลายทางของงู / บันได

3. วัตถุผู้เล่นประกอบด้วยชื่อผู้เล่นและตำแหน่งผู้เล่น

4. ในการเริ่มต้นด้วยแรนดอมฉันสร้างไทล์แบบสุ่มให้เป็นงูและบันได จากนั้นผู้ใช้จะถูกขอให้ป้อน no ของผู้เล่นแล้วหมุนลูกเต๋าไปเรื่อย ๆ จนกว่าผู้เล่นจะมาถึงไทล์ที่ 100

รหัส:

ชั้น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 + "]");
        }
    }
}

ชั้น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;
    }
}

ชั้น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;
    }

}

ชั้น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"));
    }
}

ผมคิดว่าเกมตรงไปตรงมามาก ฉันอยากรู้เป็นพิเศษว่า

1. มีวิธีที่ดีกว่าในการสร้างจำนวนงูและบันไดแบบสุ่ม

2. โครงสร้างข้อมูลที่ดีและเหมาะสมกว่าเพื่อแสดงถึง 'คณะกรรมการ'

3. หากรหัสของฉันสามารถปรับปรุงให้มีกระบวนทัศน์ / แนวปฏิบัติที่สำคัญ

Answer 1

ยินดีต้อนรับสู่ Code Review การใช้งานที่ดีและโครงสร้างที่ดีของโค้ดค้นหาคำแนะนำของฉันในบรรทัด

1. มีวิธีที่ดีกว่าในการสร้างจำนวนงูและบันไดแบบสุ่ม

วิธีการsetSnakesและsetLaddersแตกต่างกันเพียงไม่กี่บรรทัดของรหัสซึ่งเป็นตัวบ่งชี้การทำซ้ำรหัส นี่คือวิธีการ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 + "]");
    }
}

แนวทางหนึ่งคือการสร้างวิธีการสร้างตัวเลขสุ่มแล้วใช้เพื่อสร้างบันไดและงู ตัวอย่างเช่น:

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

วิธีนี้สามารถเรียกใช้nครั้งได้ขึ้นอยู่กับจำนวนบันไดที่คุณต้องการเพิ่มลงในบอร์ด วิธีนี้getRandomSegmentจะส่งกลับจุดสุ่มสองจุดระหว่าง 1 ถึง 99 นอกจากนี้ฉันยังเปลี่ยนชื่อจากsetLaddersเป็นsetRandomLadderเพื่อให้อธิบายได้ด้วยตัวเอง สุดท้ายโปรดทราบว่าหากคุณเพิ่มบันไดมากเกินไปวิธีการดังกล่าวจะไม่เสร็จสมบูรณ์ดังนั้นให้พิจารณาตรวจสอบว่ามีกระเบื้องว่างมาก่อนหรือไม่

---

1. โครงสร้างข้อมูลที่ดีและเหมาะสมกว่าเพื่อแสดงถึง 'คณะกรรมการ'

โดยใช้วิธีการก่อนหน้านี้เราสามารถลบรายการซึ่งเป็นไปได้โดยการเพิ่มoccupiedPositionsTileType.OCCUPIED

อาร์เรย์ของก็โอเคเป็นโครงสร้างข้อมูลอีกทางเลือกหนึ่งคือการใช้งานTileArrayList

---

1. หากรหัสของฉันสามารถปรับปรุงให้มีกระบวนทัศน์ / แนวปฏิบัติที่สำคัญ

• เอาต์พุตคอนโซล :

  How many players ?
  2
  Enter name for player 0
  

  ทำให้ผู้ใช้สับสนเมื่อเห็นผู้เล่น 0 เริ่มจากผู้เล่น 1

• หมายเลขมายากล : new Tile[101], secureRandom.nextInt(98)ฯลฯ สร้างค่าคงที่ตัวอย่างเช่น:

  private static final int SIZE = 101;
  

• ตัวปรับเปลี่ยน : วิธีการsetSnakesและsetLaddersควรเป็นส่วนตัวเหมือนกันสำหรับตัวแปรอินสแตนซ์ทั้งหมด

• เริ่มต้นในตัวสร้าง : BoardImplมีวิธีการinitialize()แต่และตัวสร้างว่างเปล่า คุณสามารถใช้ตัวสร้างได้โดยตรง

• Encapsulation : อินเทอร์เฟซBoardยอมรับ a PlayerกับสาธารณะsetPositionซึ่งหมายความว่าทุกคนสามารถเปลี่ยนแปลงได้ เฉพาะคณะกรรมการเท่านั้นที่ควรเปลี่ยนสถานะของผู้เล่น อินเทอร์เฟซที่ปลอดภัยกว่าอาจเป็น:

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

• หลีกเลี่ยงตัวแปรชั่วคราว : แทนที่จะเป็น

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

  คุณสามารถเขียน:

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

• รหัสกลิ่น :

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

  การส่งคืนสตริงเป็นข้อผิดพลาดได้ง่ายที่นี่ ตัวอย่างเช่นคุณสามารถส่งคืนบูลีนและในที่สุดก็เป็นข้อยกเว้น

• การทดสอบและการนำกลับมาใช้ใหม่ : การwinnerเรียกใช้เมธอดSystem.exit(0)ซึ่งทำให้ไม่สามารถทดสอบได้ นอกจากนี้ยังมีอีกมากมายSystem.out.printlnในBoardImplชั้นเรียน มันยากมากที่จะทดสอบคลาสแบบนั้น ตามหลักการแล้วคลาสBoardไม่ควรพิมพ์ไปยังผู้ใช้เพื่อการดีบักเท่านั้น mainควรจะต้องรับผิดชอบในการโต้ตอบกับผู้ใช้

Answer 2

ความสามารถในการอ่านและรูปแบบ

โปรดเรียกใช้โค้ดของคุณผ่านฟอร์แมตเตอร์เพื่อให้อ่านง่ายขึ้นและปฏิบัติตามมาตรฐาน

อันนี้อาจใช้ได้:

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

ธง?

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

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

คุณไม่ได้ใช้ตรวจสอบหรือเปลี่ยนค่าสถานะที่นี่ ลูปนี้ควรจะเป็นwhile(true)และลูปถัดไปทำสิ่งเดียวกัน

ถ้า - อื่น -

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

เมื่อคุณกำลังตรวจสอบกรณีพิเศษหลายกรณีและเปรียบเทียบกับสตริงฉันคิดว่าswitchคำสั่งเป็นวิธีที่เหมาะสม / เป็นมาตรฐานมากกว่า นอกจากนี้ยังทำให้ชัดเจนยิ่งขึ้นว่ากรณีต่างๆเป็นเอกสิทธิ์เฉพาะบุคคล

โครงสร้างข้อมูล

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

สิ่งนี้ดูเหมือนไม่จำเป็น / ผิดธรรมชาติสำหรับฉัน ฉันจะเพิ่มลงในคลาสไทล์ a boolเพื่อทำเครื่องหมายไทล์ว่าถูกครอบครองจากนั้นข้อมูลนั้นสามารถอยู่ในไทล์ / บอร์ดได้เองแทนที่จะแยกต่างหากSetสำหรับการทำบัญชี