Ik heb een probleemstelling gekregen om een Battleship-game in Java te maken.
Mijn werkende code (Spring Boot + Web) wordt hier samen met de probleemstelling geplaatst. https://github.com/ankidaemon/BattleShip
Deze vraag is voornamelijk gericht op design. Help me er alsjeblieft achter hoe kan ik het loskoppelen en geschikte ontwerppatronen toepassen.
StartGame.java – gebeld worden door 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; } } Voorbeeldinvoer
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 Regels
1. Speler1 wordt als eerste geactiveerd. Elke speler krijgt nog een kans tot (hit == succesvol).
2. Slagschepen worden horizontaal geplaatst.
3. Type-Q-schip heeft 2 raketten nodig om te worden vernietigd.
4. Type-P-schip vereist 1 raketaanval om vernietigd te worden.
Input
De eerste regel van de invoer bevat afmetingen van het slaggebied met breedte en hoogte gescheiden door een spatie.
De tweede regel heeft het aantal (B) slagschepen dat elke speler heeft.
Dan zullen in het volgende type slagschip de afmetingen (breedte en hoogte) & posities (Y-coördinaat en X-coördinaat) voor Speler-1 en vervolgens voor Speler-2 zijn gegeven gescheiden door een spatie.
En dan wordt in de volgende regel de reeks van speler-1 (gescheiden door een spatie) van raketten de doellocatiecoördinaten (Y en X) gegeven en vervolgens voor de reeks voor speler-2.
Beperkingen:
1 < = Breedte van gevechtsgebied (M) < = 9
A < = Hoogte van gevechtsgebied (N ) < = Z
1 < = Aantal slagschepen < = M * N
Type schip = {P, Q}
1 < = Breedte van slagschip < = M
A < = Hoogte van slagschip < = N
1 < = X-coördinaat van schip < = M
A < = Y-coördinaat van schip < = N
Reacties
- Je zou de regels van het spel en de commentaren kunnen toevoegen in ieder geval in de belangrijkste delen van de code. Je zou ook die kritieke delen van de code naar subroutines kunnen verplaatsen om de code veel leesbaarder te maken, hoewel het aanroepen van zoveel functies de code langzamer zou maken, maar het zal in ieder geval veel leesbaarder worden voor anderen, zodat ze later de updates beter kunnen onderhouden . Je zou de originele code en een meer modulaire / becommentarieerde code kunnen toevoegen, vergezeld van de spelregels, misschien de commentaren op die regels baseren.
- @ alt.126 hebben regels en invoer toegevoegd. StartGame leest de invoer uit een bestand, voert validatie uit en maakt BattleArea en BattleShips voor elke speler. BattleShips zijn POJO. BattleArea heeft methoden om schepen en fileMissiles te plaatsen op basis van de regels. Thnx
Antwoord
ok, laten we de handen op elkaar leggen:
Klassenaam voor uw StartGame is niet nuttig, hernoem het in een meer overeenkomende naam, ik denk zoals BattleShipGame en start het spel in plaats van je controller
BattleShipGame game = new BattleShipGame(); game.start(); het init – methode is veel te groot en het doet geen init maar doet nog meer dingen … dus laten we dat een beetje opsplitsen:
- init zou retourneer een boolean (of een
Result) die aangeeft dat init succesvol was. - init ziet eruit als “een delegate-methode wat betekent er zou heel weinig logica in moeten zitten – in plaats daarvan is het handig om het meeste werk in methoden te stoppen
- start gewoon dingen en doe geen andere dingen
- gebruik
Playerobjecten … - verplaats de spellogica uit de methode
het zou er dan zo uit kunnen zien
OPMERKING: de init-methode zou veel meer kunnen worden ingekort, maar ik denk dat ik op een goede manier aangeeft wat init eigenlijk zou moeten zijn do …
zoals hierboven vermeld, heb je de spellogica uit je init-methode gehaald en in de playGame() -methode geplaatst.
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; } de BattleShipGame zou nu op deze manier beginnen:
public void start(){ init(); Result result = playGame(); ... //do whatever you want with your result - for example print it into the console } voor je BattleShip zijn er nog enkele kwesties waarover gesproken kan worden. Ik denk dat het een heel goed idee was om een klasse Coordinate te gebruiken die er op het eerste gezicht goed uitziet. Maar u gebruikt het niet consequent. Denk na over hoe het zou zijn als u Coordinate voor uw verzending zou gebruiken in plaats van int[] dat zou maken uw code is ook gemakkelijker te lezen en de wiskunde zou veel gemakkelijker zijn. En “gebruik geen char voor uw scheepstype, maar gebruik in plaats daarvan een enum. Maar laten we eerlijk zijn, je hebt geen “positie en breedte en hoogte” wat je echt hebt is een rechthoek – dus gebruik een rechthoek!
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(); } ... } de afmeting van de rechthoek (breedte / hoogte) en het aantal levenspunten kan worden bepaald door het 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(); } } De BattleArea is nu veel gemakkelijker te gebruik, bedenk eens hoe eenvoudig je placeShips nu kunt:
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); } } } alle bovenstaande code is niet gecompileerd, dus daar kunnen enkele spelfouten zijn en veel methoden zijn nog niet eens geïmplementeerd (zoals Rectangle.contains() of andere).
samenvatting
maar laat “s kijk naar wat we nu hebben:
- je kunt het scheepstype vrij gemakkelijk wijzigen zonder elke code !!! (je hoeft alleen maar een ander scheepstype toe te voegen in
ShipType) - je hebt de complexiteit van je code erg verminderd, je hoeft niet “Geen gevaarlijke berekening ionen.
- je hebt verschillende zorgen, de objecten doen nu wat ze moeten doen
- je zou gemakkelijk je code kunnen veranderen voor een andere speler (spel met drie spelers)
- je zou nu je code kunnen testen
Reacties
- nou ik weet dat er hier nog zoveel problemen zijn dat ik niet kon ‘ kan ze niet allemaal aan – dit antwoord stond helemaal bovenaan en ging niet in op details
Antwoord
Om te ontkoppelen moet je ervoor zorgen dat je functies elkaar niet hoeven te bellen om hun werk te doen. Indien mogelijk zijn de enige functies die anderen zouden moeten aanroepen de stuurprogrammafuncties van elk subsysteem die bedoeld zijn om te worden aangeroepen zoals de API-interface.
Bedenk hoe u veel code zou moeten porten alleen voor het toevoegen een handige functie als die functie andere functies aanroept of variabelen gebruikt uit andere grote subsystemen. Als u extra moeite doet om die functie op een manier te implementeren dat deze niet van iets anders afhankelijk is, zelfs als het eruit ziet als gedupliceerde code, kunt u deze individueel naar een ander programma overzetten, en zelfs meer als u laat het niet afhangen van taalkenmerken of bibliotheekkenmerken die niet aanwezig zijn op elke compiler en programmeertaal die je gebruikt om het mogelijk te maken om elke code die je schrijft over te dragen naar elke gewenste omgeving , dat is wat s genaamd ontkoppeling .
Zoals je kunt zien, kan ontkoppeling worden toegepast op de compiler, taal, systeemomgeving, functies en subsysteemniveaus. Het kan gaan om het dupliceren van code en het herschrijven om zelfstandige, afhankelijkheidsloze routines te hebben. Het kan ook betekenen dat u meer algemeen gestandaardiseerde functies gebruikt om de code draagbaarder te maken, en het kan ook nodig zijn dat u zelf werkt aan het implementeren of porteren van ontbrekende functionaliteit naar al uw programmeer- / systeemomgevingen, zodat het systeem / de taal / de compiler u gebruik, heb je altijd dezelfde functionaliteit beschikbaar.
Over ontwerppatronen.
Als je je code zeer herbruikbaar wilt maken en als je wilt dat het tientallen jaren meegaat, zou je de low-level benadering van CPU-assemblageprogrammering kunnen gebruiken.
Denk na over een taak of microtaak die je wilt uitvoeren op een manier die altijd hetzelfde type parameters en dat zal altijd op exact dezelfde manier een resultaat opleveren.
Geef het dan een ZEER specifieke naam aan deze routine. Dit zal een opcode zijn, een instructie, geïmplementeerd als een functie / subroutine, die je kunt hergebruiken, net als elke andere native CPU-instructie. Deze manier van ontwerpen van de code is zeer herbruikbaar en stabiel. Als je een variatie wilt in wat je moet doen, voeg je gewoon een nieuwe opcode-functie toe in plaats van de vorige geldige functionaliteit te vernietigen.
Door dit in het hele programma toe te passen, omdat de belangrijkste ontwerpbenadering de code nog strikter kan maken gemakkelijker te volgen.
Reacties
- Bedankt, @ alt.126. Hoewel dit een algemene oplossing is (+1), hoopte ik eigenlijk meer op Java-ontwerppatronen / minder objecten maken / overtollige code verwijderen enz.