Mi è stato dato un problema per creare un gioco Battleship in java.
Il mio codice di lavoro (Spring Boot + Web) viene inserito qui insieme allistruzione del problema. https://github.com/ankidaemon/BattleShip
Questa domanda è principalmente incentrata sul design, per favore aiutami a capire come posso renderlo disaccoppiato e applicare modelli di progettazione adeguati.
StartGame.java – essere chiamato dal controller
@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; } } Esempio di input
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 Regole
1. Player1 si attiva per primo. Ogni giocatore avrà unaltra possibilità fino a (hit == successo).
2. Le corazzate saranno posizionate orizzontalmente.
3. La nave di tipo Q richiede 2 missili colpiti per essere distrutta.
4. La nave di tipo P richiede 1 missile per essere distrutta.
Input
La prima riga dellinput contiene le dimensioni dellarea di battaglia con larghezza e altezza separate da uno spazio.
La seconda riga avrà il numero (B) di corazzate di ogni giocatore.
Quindi nella riga successiva tipo di corazzata, le dimensioni (larghezza e altezza) & posizioni (coordinata Y e coordinata X) per Player-1 e poi per Player-2 saranno dato separato da spazio.
E poi nella riga successiva verranno fornite le coordinate della posizione del bersaglio del missile (separate da spazio) nella riga successiva e poi per la sequenza del giocatore-2.
Vincoli:
1 < = Larghezza dellarea di battaglia (M) < = 9
A < = Altezza dellarea di battaglia (N ) < = Z
1 < = Numero di corazzate < = M * N
Tipo di nave = {P, Q}
1 < = Larghezza della corazzata < = M
A < = Altezza della corazzata < = N
1 < = Coordinata X della nave < = M
A < = Coordinata Y della nave < = N
Commenti
- Potresti aggiungere le regole del gioco e i commenti almeno nelle parti più importanti del codice. Puoi anche spostare quelle parti critiche del codice in subroutine per rendere il codice molto più leggibile, anche se chiamare così tante funzioni renderebbe il codice più lento, ma almeno diventerà molto più leggibile per gli altri in modo che in seguito possano mantenere meglio gli aggiornamenti . Potresti aggiungere il codice originale e un codice più modulare / commentato accompagnato dalle regole del gioco, magari basando i commenti su quelle regole.
- @ alt.126 hanno aggiunto regole e input. StartGame legge linput da un file, esegue la convalida e crea BattleArea e BattleShips per ogni giocatore. Le navi da battaglia sono POJO. BattleArea ha metodi per posizionare navi e fileMissiles in base alle regole. Grazie
Risposta
ok mettiamo le mani su:
Nome della classe per il tuo StartGame non è utile, rinominalo con un nome più corrispondente, penso come BattleShipGame e avvia il gioco invece dal controller
BattleShipGame game = new BattleShipGame(); game.start(); init – il metodo è decisamente troppo grande e non fa init ma fa anche più cose … quindi scomponiamolo un po :
- init dovrebbe restituisce un booleano (o un
Result) che indica che linizializzazione ha avuto esito positivo. - sembra che init “è un metodo delegato che significa ci dovrebbe essere pochissima logica insde – invece è utile dedicare la maggior parte del lavoro ai metodi
- solo inizializzare le cose e non fare altre cose
- usare
Playeroggetti … - sposta la logica del gioco fuori dal metodo
potrebbe essere così
NOTA: il metodo init potrebbe essere accorciato molto di più, ma penso di indicare in modo corretto cosa init dovrebbe veramente fare …
come menzionato sopra, hai spostato la logica di gioco fuori dal tuo metodo init e lhai inserita nel metodo 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; } BattleShipGame inizierebbe ora in questo modo:
public void start(){ init(); Result result = playGame(); ... //do whatever you want with your result - for example print it into the console } per la tua BattleShip ci sono altri problemi di cui si può parlare. Penso che sia stata unottima idea usare una classe Coordinate che a prima vista sembra buona. Ma non lo usi di conseguenza. Pensa a come sarebbe se usassi Coordinate per la spedizione invece di int[] che farebbe anche il tuo codice è più facile da leggere e la matematica sarebbe molto più semplice. E non usare char per il tuo tipo di nave, usa invece un enum. Ma siamo onesti, non hai una “posizione, larghezza e altezza” quello che hai veramente è un rettangolo, quindi usa un rettangolo!
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 dimensione del rettangolo (larghezza / altezza) e la quantità di lifepoint possono essere determinate da ShipType
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(); } } BattleArea è ora molto più facile da usa, pensa a quanto semplice puoi placeShips ora:
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); } } } tutto il codice sopra non è stato compilato quindi lì potrebbero esserci degli errori di ortografia e molti metodi non sono nemmeno ancora implementati (come Rectangle.contains() o altri).
riepilogo
ma lascia “Guarda quello che abbiamo ora:
- puoi cambiare il tipo di nave abbastanza facilmente senza modificare qualsiasi codice !!! (devi semplicemente aggiungere un altro tipo di nave in
ShipType) - hai ridotto la complessità del tuo codice molto lontano, non “Non ho calcoli pericolosi ioni.
- hai preoccupazioni separate, gli oggetti ora fanno quello che dovrebbero fare
- potresti facilmente cambiare il tuo codice per un altro giocatore (gioco a tre giocatori)
- potresti testare il tuo codice adesso
Commenti
- beh so che ci sono ancora così tanti problemi qui che non potrei ‘ t gestirli tutti: questa risposta era nella vista dallalto senza entrare nei dettagli
Risposta
Per disaccoppiare devi assicurarti che le tue funzioni non abbiano bisogno di chiamarsi a vicenda per fare il loro lavoro. Se possibile, le uniche funzioni che dovrebbero chiamare altre sono le funzioni driver di ogni sottosistema che devono essere chiamate come linterfaccia API.
Pensa a come dovresti portare molto codice solo per laggiunta una funzione utile se quella funzione chiama altre funzioni o usa variabili da altri grandi sottosistemi. Se fai uno sforzo ulteriore per implementare quella funzione in modo che non dipenda da nientaltro, anche se sembra codice duplicato, sarai in grado di portarlo individualmente su un altro programma, e anche di più se non farlo dipendere dalle caratteristiche del linguaggio o dalle caratteristiche della libreria che non sono presenti in ogni singolo compilatore e linguaggio di programmazione che usi per rendere possibile il porting di qualsiasi codice che scrivi in qualsiasi ambiente di cui hai bisogno , ecco cosa s chiamato disaccoppiamento .
Come puoi vedere, il disaccoppiamento può essere applicato a livello di compilatore, linguaggio, ambiente di sistema, funzioni e sottosistema. Potrebbe comportare la duplicazione del codice e la riscrittura per avere routine autonome e prive di dipendenze. Potrebbe anche implicare lutilizzo di funzionalità più standardizzate per rendere il codice più portabile e potrebbe anche essere necessario che tu lavori sullimplementazione o il porting delle funzionalità mancanti in tutti i tuoi ambienti di programmazione / sistema in modo che indipendentemente dal sistema / linguaggio / compilatore tu utilizzare, avrai sempre la stessa funzionalità disponibile.
Informazioni sui modelli di progettazione.
Se vuoi rendere il tuo codice altamente riutilizzabile e se vuoi che duri per decenni, puoi utilizzare lapproccio di basso livello della programmazione dellassemblaggio della CPU.
Pensa a unattività o a una microtask che desideri eseguire in un modo che richieda sempre lo stesso tipo di parametri e questo restituirà sempre un risultato esattamente allo stesso modo.
Quindi dagli un nome MOLTO specifico a questa routine. Questo sarà un codice operativo, unistruzione, implementata come funzione / subroutine, che puoi riutilizzare proprio come qualsiasi altra istruzione nativa della CPU. Questo modo di progettare il codice è altamente riutilizzabile e stabile. Se vuoi una variazione su cosa fare, aggiungi semplicemente una nuova funzione di codice operativo invece di distruggere la precedente funzionalità valida.
Applicare questo in tutto il programma come approccio di progettazione principale può rendere il codice ancora più rigorosamente strutturato più facile da seguire.
Commenti
- Grazie, @ alt.126. Sebbene questa sia una soluzione generalizzata (+1), in realtà speravo di più sui modelli di progettazione Java / creazione di meno oggetti / rimozione di codice ridondante ecc.