Jag har fått ett problemuttalande för att skapa ett Battleship-spel i java.
Min arbetskod (Spring Boot + Web) finns här tillsammans med problemförklaringen. https://github.com/ankidaemon/BattleShip
Denna fråga är främst inriktad på design, snälla hjälp mig att räkna ut, hur kan jag göra det frikopplat och tillämpa lämpliga designmönster.
StartGame.java – ringa från styrenheten
@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; } } Koordinera.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; } } Exempelinmatning
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 Regler
1. Player1 avfyrar först. Varje spelare får en ny chans till (hit == lyckad).
2. Slagskepp kommer att placeras horisontellt.
3. Typ-Q-fartyg kräver 2 missiler som träffas för att förstöras.
4. Type-P-fartyg kräver 1 missilträff för att förstöras.
Inmatning
Ingångens första rad innehåller mått på stridsområdet med bredd och höjd åtskilda av rymden.
Andra raden kommer att ha antal (B) slagskepp varje spelare har.
Sedan i nästa rad slagskeppstyp kommer dimensioner (bredd och höjd) & positioner (Y-koordinat och X-koordinat) för Player-1 och sedan för Player-2 att vara ges åtskilda av utrymme.
Och i nästa rad kommer Player-1-sekvensen (åtskilda av mellanslag) av missiler målplatskoordinater (Y och X) att ges och sedan för sekvens för Player-2.
Begränsningar:
1 < = Bredd på stridsområdet (M) < = 9
A < = Stridsområdets höjd (N ) < = Z
1 < = Antal slagskepp < = M * N
Fartygstyp = {P, Q}
1 < = Slagskeppets bredd < = M
A < = Slagskeppets höjd < = N
1 < = X-koordinat för fartyg < = M
A < = Y-koordinat för fartyg < = N
Kommentarer
- Du kan lägga till spelreglerna och kommentarerna åtminstone i de viktigaste delarna av koden. Du kan också flytta de kritiska delarna av koden till underrutiner för att göra koden mycket mer läsbar, även om att ringa så många funktioner skulle göra koden långsammare, men åtminstone blir den mycket mer läsbar för andra så att de senare kan behålla uppdateringarna bättre . Du kan lägga till den ursprungliga koden och en mer modulär / kommenterad kod åtföljd av spelreglerna, kanske basera kommentarerna på dessa regler.
- @ alt.126 har lagt till regler och input. StartGame läser inmatningen från en fil, gör validering och skapar BattleArea och BattleShips för varje spelare. BattleShips är POJO. BattleArea har metoder för att placera fartyg och filmissiler baserat på reglerna. Thnx
Svar
ok låter händerna på:
Klassnamn för din StartGame är inte till hjälp, byt namn på det till ett mer matchande namn, jag tycker att BattleShipGame och starta istället spelet från din kontroller
BattleShipGame game = new BattleShipGame(); game.start(); init – metoden är långt för stor och den gör ingen init men gör ännu fler saker … så låt oss bryta ner det lite:
- init ska returnera en boolean (eller en
Result) som indikerar att init lyckades. - init ser ut som den” sa delegera metod vilket betyder det borde vara väldigt lite logiskt – istället är det bra att lägga mest arbete på metoder
- bara initiera saker och inte göra några andra saker
- använd
Playerobjekt … - flytta spellogiken ur metoden
det kan se ut så här
OBS: initmetoden kan förkortas mycket mer, men jag tror att jag på ett bra sätt påpekar vad init verkligen borde gör …
som nämnts ovan har du flyttat spellogiken från din init-metod och placerat den i playGame() -metoden.
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 skulle börja nu på detta sätt:
public void start(){ init(); Result result = playGame(); ... //do whatever you want with your result - for example print it into the console } för ditt BattleShip finns det några fler utgivningar som man kan prata om. Jag tycker att det var en mycket bra idé att använda en klass Coordinate som ser bra ut vid första anblicken. Men du använder det inte konsekvent. Tänk på hur det skulle vara om du använde Coordinate för att du skickade istället för int[] som skulle göra din kod också lättare att läsa och matematiken skulle vara mycket lättare. Och använd inte en char för din fartygstyp, använd enum istället. Men låt oss vara ärliga, du har inte en ”position och bredd och höjd”. Det du verkligen har är en rektangel – så använd en rektangel!
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(); } ... } Rektangelns dimension (bredd / höjd) och mängden livspoäng kan bestämmas av 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 är nu mycket lättare att använd, tänk på hur enkelt du kan placeShips nu:
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); } } } all kod ovan har inte sammanställts så där kan vara vissa stavfel och många metoder har inte ens implementerats än (som Rectangle.contains() eller andra).
sammanfattning
men låt ”titta på vad vi har nu:
- du kan ändra fartygstypen ganska enkelt utan att ändra någon -kod !!! (du måste helt enkelt lägga till en annan fartygstyp i
ShipType) - du har minskat komplexiteten i din kod väldigt långt, du don ”t har farlig beräkning joner.
- du har separata problem, objekten gör nu vad de ska göra
- du kan enkelt ändra din kod för en annan spelare (tre spelarspel)
- du kan testa din kod nu
Kommentarer
- ja jag vet att det fortfarande finns så många problem här att jag inte kunde ’ t hantera dem alla – det här svaret var högst uppifrån och kom inte in i några detaljer
Svar
För att koppla loss måste du se till att dina funktioner inte behöver ringa varandra för att göra sitt arbete. Om möjligt är de enda funktionerna som ska ringa andra drivrutinsfunktionerna för varje delsystem som är avsedda att kallas som API-gränssnittet.
Tänk på hur du skulle behöva porta mycket kod bara för att lägga till en användbar funktion om den funktionen anropar andra funktioner eller använder variabler från andra stora delsystem. Om du gör ytterligare ansträngningar för att implementera den funktionen på ett sätt som inte beror på något annat, även om det ser ut som duplicerad kod, kommer du att kunna porta den individuellt till ett annat program, och ännu mer om du Låt det inte bero på språkfunktioner eller biblioteksfunktioner som inte finns på varje enskild kompilator och programmeringsspråk som du använder för att göra det möjligt att porta vilken kod du skriver till vilken miljö du behöver , det är vad ” kallas frikoppling .
Som du kan se kan frikoppling tillämpas på kompilatorn, språket, systemmiljön, funktioner och delsystemnivåer. Det kan handla om att duplicera kod och skriva om att ha fristående, beroendeframkallande rutiner. Det kan också innebära att du använder mer standardiserade funktioner för att göra koden mer bärbar, och det kan också behöva att du arbetar med att implementera eller portera saknad funktionalitet till alla dina programmerings- / systemmiljöer så att oavsett system / språk / kompilator du användning har du alltid samma funktionalitet tillgänglig.
Om designmönster.
Om du vill göra din kod mycket återanvändbar och om du vill att den ska pågå i årtionden kan du använda den låga nivån när det gäller programmering av CPU-enheter.
Tänk på en uppgift eller mikrotask som du vill utföra på ett sätt som alltid tar samma typ av parametrar och som alltid returnerar ett resultat på exakt samma sätt.
Ge det sedan ett MYCKET specifikt namn på denna rutin. Detta kommer att vara en opcode, en instruktion, implementerad som en funktion / underrutin, som du kan återanvända precis som alla andra inbyggda CPU-instruktioner. Detta sätt att utforma koden är mycket återanvändbart och stabilt. Om du vill ha en variation i vad du ska göra lägger du bara till en ny opkodfunktion istället för att förstöra den tidigare giltiga funktionaliteten.
Om du använder detta i hela programmet som huvuddesign kan du göra koden mer strikt strukturerad än lättare att följa.
Kommentarer
- Tack, @ alt.126. Även om detta är en generaliserad lösning (+1) hoppades jag faktiskt mer på Java-designmönster / gjorde mindre objekt / tog bort överflödig kod etc.