Me han dado una declaración de problema para crear un juego Battleship en Java.
Mi código de trabajo (Spring Boot + Web) se coloca aquí junto con la declaración del problema. https://github.com/ankidaemon/BattleShip
Esta pregunta se centra principalmente en el diseño. Ayúdame a averiguar cómo ¿Puedo desacoplarlo y aplicar patrones de diseño adecuados?
StartGame.java – recibiendo llamada desde el controlador
@Component public class StartGame { private static final Logger logger = LoggerFactory.getLogger(StartGame.class); public String init(File inputFile) throws FileNotFoundException, InputException { // TODO Auto-generated method stub ArrayList<BattleShips> p1s = new ArrayList<BattleShips>(); ArrayList<BattleShips> p2s = new ArrayList<BattleShips>(); int areaWidth = 0; int areahight = 0; ArrayList<Coordinate> player1missiles = null; ArrayList<Coordinate> player2missiles = null; try{ Scanner sc = new Scanner(inputFile); areaWidth = sc.nextInt(); if(areaWidth>9 || areaWidth<1){ raiseException("Supplied area width is invalid.",sc); } areahight = sc.next().toUpperCase().charAt(0) - 64; if(areahight>25 || areahight<0){ raiseException("Supplied area height is invalid.",sc); } sc.nextLine(); int noOfships = sc.nextInt(); if(noOfships>areahight*areaWidth || noOfships<1){ raiseException("Supplied no of ships is invalid.",sc); } sc.nextLine(); for (int j = 0; j < noOfships; j++) { char typeOfShip = sc.next().toUpperCase().charAt(0); if(typeOfShip!="P" && typeOfShip!="Q"){ raiseException("Supplied type of ship is invalid.",sc); } int shipWidth = sc.nextInt(); if(shipWidth>areaWidth || shipWidth<0){ raiseException("Supplied ship width is invalid.",sc); } int shiphight = sc.nextInt(); if(shiphight>areahight || shiphight<0){ raiseException("Supplied ship height is invalid.",sc); } BattleShips ship; for (int i = 0; i <= 1; i++) { char[] locCharArr = sc.next().toUpperCase().toCharArray(); int[] loc = new int[2]; loc[0] = locCharArr[0] - 65; loc[1] = locCharArr[1] - 49; if(loc[0]>areahight || loc[0]<0 || loc[1]>areaWidth || loc[1]<0){ raiseException("Supplied ship location is invalid.",sc); } ship = new BattleShips(shipWidth, shiphight, typeOfShip, loc); if (i % 2 == 0) p1s.add(ship); else p2s.add(ship); } sc.nextLine(); } player1missiles = returnMissileCoordinates(sc.nextLine()); player2missiles = returnMissileCoordinates(sc.nextLine()); sc.close(); }catch(InputMismatchException e){ throw new InputException("Invalid Input supplied.",ErrorCode.INVALIDINPUT); } BattleArea player1 = new BattleArea("player1", areaWidth, areahight, p1s); BattleArea player2 = new BattleArea("player2", areaWidth, areahight, p2s); player1.placeShips(); player2.placeShips(); while (!player1.isLost() && !player2.isLost()) { for (int i = 0; i < player1missiles.size();) { Coordinate c = player1missiles.get(i); while (player1.fireMissile(c, player2)) { player1missiles.remove(i); if (i < player1missiles.size()) { c = player1missiles.get(i); } else break; } if (player1missiles.size() > 0) { player1missiles.remove(i); } break; } for (int j = 0; j < player2missiles.size();) { Coordinate c = player2missiles.get(j); while (player2.fireMissile(c, player1)) { player2missiles.remove(j); if (j < player2missiles.size()) { c = player2missiles.get(j); } else break; } if (player2missiles.size() > 0) { player2missiles.remove(j); } break; } } if (player1.isLost()) { logger.info("-------------------------"); logger.info("Player 2 has Won the Game"); logger.info("-------------------------"); return "Player 2 has Won the Game"; } else { logger.info("-------------------------"); logger.info("Player 1 has Won the Game"); logger.info("-------------------------"); return "Player 1 has Won the Game"; } } private static ArrayList<Coordinate> returnMissileCoordinates(String nextLine) { // TODO Auto-generated method stub ArrayList<Coordinate> tmp = new ArrayList<Coordinate>(); String[] arr = nextLine.split("\\ "); Coordinate tmpC; for (String s : arr) { char[] charArr = s.toCharArray(); tmpC = new Coordinate(charArr[1] - 49, charArr[0] - 65); tmp.add(tmpC); } return tmp; } private void raiseException(String message, Scanner sc) throws InputException { sc.close(); throw new InputException(message, ErrorCode.INVALIDINPUT); } } BattleArea.java
public class BattleArea { private static final Logger logger = LoggerFactory.getLogger(BattleArea.class); private String belongsTo; private int width,height; private ArrayList<BattleShips> battleShips; private Set<Coordinate> occupied=new TreeSet<Coordinate>(); private int[][] board=null; private boolean lost=false; public BattleArea(String belongsTo, int width, int height, ArrayList<BattleShips> battleShips) { super(); this.belongsTo = belongsTo; this.width = width; this.height = height; this.battleShips = battleShips; this.board=new int[this.width][this.height]; } public void placeShips(){ for(BattleShips ship:this.battleShips){ int x=ship.getLocation()[1]; int y=ship.getLocation()[0]; if(ship.getWidth()+x>this.width || ship.getHeight()+y>this.height){ logger.error("Coordinate x-"+x+" y-"+y+" for "+this.belongsTo+" is not avilable."); throw new ProhibitedException("Ship cannot be placed in this location.",ErrorCode.OUTOFBATTLEAREA); }else{ Coordinate c=new Coordinate(x, y); if(occupied.contains(c)){ logger.error("Coordinate x-"+c.getX()+" y-"+c.getY()+" for "+this.belongsTo+" is already occupied."); throw new ProhibitedException("Ship cann"t be placed in this location.",ErrorCode.ALREADYOCCUPIED); }else{ Coordinate tempC; for(int i=x;i<ship.getWidth()+x;i++){ for(int j=y;j<ship.getHeight()+y;j++){ logger.debug("Placing at x-"+i+" y-"+j+" for "+this.belongsTo); tempC=new Coordinate(i, j); occupied.add(tempC); if(ship.getTypeOfShip()=="P"){ board[i][j]=1; }else if(ship.getTypeOfShip()=="Q"){ board[i][j]=2; } } } } } } } public boolean fireMissile(Coordinate c, BattleArea enemyBattleArea){ int x=c.getX(); int y=c.getY(); logger.info("Firing at "+enemyBattleArea.belongsTo+" x-"+x+" y-"+y+" :"); if(enemyBattleArea.board[x][y]!=0){ if(enemyBattleArea.board[x][y]==-1){ logger.debug("Already blasted!"); return false; } else if(enemyBattleArea.board[x][y]==1){ Coordinate temp=new Coordinate(x,y); enemyBattleArea.occupied.remove(temp); enemyBattleArea.board[x][y]=-1; if(enemyBattleArea.occupied.size()==0){ enemyBattleArea.setLost(true); } logger.debug("Suucessfully blasted!!"); return true; }else{ enemyBattleArea.board[x][y]=enemyBattleArea.board[x][y]-1; logger.debug("Half life left!!"); return true; } }else{ logger.debug("Missed"); return false; } } public boolean isLost() { return lost; } public void setLost(boolean lost) { this.lost = lost; } } BattleShips.java
public class BattleShips { private int width,height; private char typeOfShip; private int[] location; public BattleShips(int width, int height, char typeOfShip, int[] loc) { super(); this.width = width; this.height = height; this.typeOfShip = typeOfShip; this.location = loc; } public int getWidth() { return width; } public int getHeight() { return height; } public char getTypeOfShip() { return typeOfShip; } public int[] getLocation() { return location; } } Coordinate.java
public class Coordinate implements Comparable<Coordinate> { private int x,y; public Coordinate(int x, int y) { super(); this.x = x; this.y = y; } @Override public String toString() { return "Coordinate [x=" + x + ", y=" + y + "]"; } @Override public int compareTo(Coordinate o) { // TODO Auto-generated method stub if(this.x==o.x && this.y==o.y) return 0; else if(this.x<o.x && this.y<o.y) return -1; else return 1; } public int getX() { return x; } public int getY() { return y; } } Entrada de muestra
5 E 2 Q 1 1 A1 B2 P 2 1 D4 C3 A1 B2 B2 B3 A1 B2 B3 A1 D1 E1 D4 D4 D5 D5 Reglas
1. El jugador1 disparará primero. Cada jugador tendrá otra oportunidad hasta (hit == exitoso).
2. Los acorazados se colocarán horizontalmente.
3. Los barcos de tipo Q requieren 2 misiles impactados para ser destruidos.
4. La nave de tipo P requiere 1 impacto de misil para ser destruida.
Entrada
La primera línea de la entrada contiene las dimensiones del área de batalla que tienen ancho y alto separados por espacio.
La segunda línea tendrá el número (B) de acorazados que tiene cada jugador.
Luego, en la siguiente línea, tipo de acorazado, las dimensiones (ancho y alto) & posiciones (coordenada Y y coordenada X) para el Jugador-1 y luego para el Jugador-2 serán dado separados por espacio.
Y luego, en la siguiente línea, se dará la secuencia del Jugador-1 (separada por un espacio) de las coordenadas de ubicación del objetivo de los misiles (Y y X) y luego para la secuencia del Jugador-2.
Restricciones:
1 < = Ancho del área de batalla (M) < = 9
A < = Altura del área de batalla (N ) < = Z
1 < = Número de acorazados < = M * N
Tipo de barco = {P, Q}
1 < = Ancho del acorazado < = M
A < = Altura del acorazado < = N
1 < = Coordenada X del barco < = M
A < = Coordenada Y del barco < = N
Comentarios
- Puedes agregar las reglas del juego y los comentarios al menos en las partes más importantes del código. También podría mover esas partes críticas del código a subrutinas para hacer el código mucho más legible, aunque llamar a tantas funciones haría que el código fuera más lento, pero al menos será mucho más legible para otros para que luego puedan mantener mejor las actualizaciones. . Puede agregar el código original y un código más modular / comentado acompañado de las reglas del juego, tal vez basando los comentarios en esas reglas.
- @ alt.126 ha agregado reglas y entradas. StartGame lee la entrada de un archivo, realiza la validación y crea BattleArea y BattleShips para cada jugador. Los BattleShips son POJO. BattleArea tiene métodos para colocar barcos y fileMissiles según las reglas. Thnx
Respuesta
ok, vamos a poner manos a la obra:
Nombre de clase para su StartGame no es útil, cámbiele el nombre a un nombre más coincidente, creo que BattleShipGame y inicie el juego en su lugar desde su controlador
BattleShipGame game = new BattleShipGame(); game.start(); el init – el método es demasiado grande y no hace init pero hace aún más cosas … así que analicemos eso un poco:
- init debería devuelve un valor booleano (o
Result) que indica que init fue exitoso. - init parece ser «un método delegado que significa Debe haber muy poca lógica en el interior; en cambio, es útil poner la mayor parte del trabajo en los métodos
- solo iniciar las cosas y no hacer ninguna otra cosa
- usar
Playerobjetos … - mover la lógica del juego fuera del método
podría verse así entonces
NOTA: el método init podría reducirse y mucho más, pero creo que señalo de buena manera lo que init debería realmente do …
como se mencionó anteriormente, ha sacado la lógica del juego de su método init y la ha puesto en el método playGame().
public Result playGame(){ Result result = new Result(); Scores score = new Score(); while (!player1.isLost() && !player2.isLost()) { for (int i = 0; i < player1missiles.size();) { ... } } result.setWinner(playerOne); result.setScore(scores); return result; } el BattleShipGame comenzaría ahora de esta manera:
public void start(){ init(); Result result = playGame(); ... //do whatever you want with your result - for example print it into the console } para tu BattleShip hay algunos problemas más de los que se puede hablar. Creo que fue una muy buena idea usar una clase Coordinate que se ve bien a primera vista. Pero no lo usa en consecuencia. Piense en cómo sería si usara Coordinate para su envío en lugar de int[] que haría su código también es más fácil de leer y las matemáticas serían mucho más fáciles. Y no use un char para su tipo de envío, use una enumeración en su lugar. Pero seamos honestos, no tienes una «posición, ancho y alto», lo que realmente tienes es un rectángulo, ¡así que usa un rectángulo!
public class BattleShips { private ShipType shipType; private Rectangle bounds; private int lifePoints; public BattleShips(ShipType typeOfShip, Coordinate pos) { super(); shipType = typeOfShip; bounds = new Rectangle(shipType.getDimension, pos); lifePoints = shipType.getLifePoints(); } public Rectangle getBounds() { return bounds(); } ... } la dimensión del rectángulo (ancho / alto) y la cantidad de puntos de vida pueden ser determinadas por el tipo de barco
public Enum Shiptype { DESTROYER(2,4,2), SUBMARINE(1,3,1), ...; //don"t use shiptype P or shiptype Q private final Dimension dimension; final int lifePoints; public ShipType(int w, int h, int life){ dimension = new Dimension(w,h); lifePoints = life; } public Dimension getDimension(){ return dimension; } public int getLifePoints(){ return lifePoints(); } } El BattleArea ahora es mucho más fácil de use, piense en lo simple que puede placeShips ahora:
public class BattleArea { private Player owner; private Rectangle boardBounds; private List<BattleShips> battleShips; private List<Coordinates> board; public BattleArea(Player owner, Rectangle bounds, List<BattleShips> battleShips) { super(); this.owner = owner; this.dimension = dimension; this.battleShips = battleShips; board = createBoard(); } public void placeShips(){ List<BattleShip> placedShips = new ArrayList<>(); for(BattleShips ship:this.battleShips){ Bound shipBounds = ship.getBounds(); if(!boardBounds.contains(shipBounds)){ throw new ProhibitedException( "Ship cannot be placed in this location.",ErrorCode.OUTOFBATTLEAREA); } for (BattleShip placedShip: placedShips){ if (bounds.intersects(placedShip.getBounds()){ throw new ProhibitedException( "Ship cann"t be placed in this location.",ErrorCode.ALREADYOCCUPIED); } } placedShips.add(battleShip); } } public boolean fireMissile(Coordinate c, BattleArea enemyBattleArea){ BattleShip shipAt = enemyBattleArea.getShipAt(c); if(shipAt == null){ return false; }else{ handleDamge(shipAt, enemyBattleArea); return true; } } private void handleDamage(BattleShip opponent, BattleArea area){ int lifePointsLeft = opponent.getLifePoints() - 1; //hardcoded damage (that"s bad) if(lifPoints > 0){ //Log damage done }else{ //log destroyed area.removeBattleShip(opponent); } } } todo el código anterior no se ha compilado, por lo que puede haber algunos errores de ortografía y muchos métodos ni siquiera se han implementado todavía (como Rectangle.contains() u otros).
resumen
pero dejemos Mira lo que tenemos ahora:
- puedes cambiar el tipo de barco con bastante facilidad sin modificar cualquier código !!! (simplemente tiene que agregar otro tipo de envío en
ShipType) - ha reducido la complejidad de su código mucho, no «No tengo cálculos peligrosos
- tiene preocupaciones separadas, los objetos ahora hacen lo que se supone que deben hacer
- fácilmente podría cambiar su código para otro jugador (juego de tres jugadores)
- puedes probar tu código ahora
Comentarios
- bueno, sé que todavía hay tantos problemas aquí que no pude ‘ t manejarlos todos – esta respuesta estaba en la vista superior sin entrar en detalles
Respuesta
Para desacoplar debes asegurarte de que tus funciones no necesitan llamarse entre sí para hacer su trabajo. Si es posible, las únicas funciones que deberían llamar a otros son las funciones de controlador de cada subsistema que están destinadas a llamarse como la interfaz API.
Piense en cómo necesitaría transferir una gran cantidad de código solo para agregar una función útil si esa función llama a otras funciones o usa variables de otros grandes subsistemas. Si hace un esfuerzo adicional para implementar esa función de una manera que no dependa de nada más, incluso si parece un código duplicado, podrá transferirlo individualmente a otro programa, e incluso más si no lo haga depender de las características del lenguaje o las características de la biblioteca que no están presentes en cada compilador y lenguaje de programación que utiliza para hacer posible la transferencia de cualquier código que escriba a cualquier entorno que necesite , eso es lo que s llamado desacoplamiento .
Como puede ver, el desacoplamiento se puede aplicar al compilador, lenguaje, entorno del sistema, funciones y niveles de subsistema. Puede implicar la duplicación de código y la reescritura para tener rutinas independientes sin dependencia. También podría implicar el uso de funciones más estandarizadas para hacer que el código sea más portátil, y también podría necesitar que trabaje en implementar o portar la funcionalidad faltante a todos sus entornos de programación / sistema para que no importa el sistema / lenguaje / compilador utilizar, siempre tendrá la misma funcionalidad disponible.
Acerca de los patrones de diseño.
Si desea que su código sea altamente reutilizable y si desea que dure décadas, puede usar el enfoque de bajo nivel de la programación de ensamblaje de CPU.
Piense en una tarea o microtarea que desee realizar de una manera que siempre requiera el mismo tipo de parámetros y que siempre devolverán un resultado exactamente de la misma manera.
Entonces dale un nombre MUY específico a esta rutina. Este será un código de operación, una instrucción, implementada como una función / subrutina, que puede reutilizar como cualquier otra instrucción de CPU nativa. Esta forma de diseñar el código es altamente reutilizable y estable. Si desea una variación en lo que debe hacer, simplemente agregue una nueva función de código de operación en lugar de destruir la funcionalidad válida anterior.
Aplicar esto en todo el programa como el enfoque de diseño principal puede hacer que el código esté aún más estructurado estrictamente. más fácil de seguir.
Comentarios
- Gracias, @ alt.126. Si bien esta es una solución generalizada (+1), en realidad esperaba más en los patrones de diseño de Java / hacer menos objetos / eliminar código redundante, etc.