Otrzymałem opis problemu dotyczący stworzenia gry Battleship w Javie.
Mój działający kod (Spring Boot + Web) jest umieszczony tutaj wraz z opisem problemu. https://github.com/ankidaemon/BattleShip
To pytanie dotyczy głównie projektu. Pomóż mi dowiedzieć się, jak czy mogę go rozdzielić i zastosować odpowiednie wzorce projektowe.
StartGame.java – wywołanie z kontrolera
@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; } } Przykładowe dane wejściowe
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 Reguły
1. Gracz 1 zostanie uruchomiony jako pierwszy. Każdy gracz będzie miał kolejną szansę, aż (hit == udane).
2. Pancerniki zostaną ustawione poziomo.
3. Okręt typu Q wymaga trafienia 2 pocisków, aby zostać zniszczony. Okręt typu P wymaga 1 trafienia pociskiem, aby zostać zniszczony.
Dane wejściowe
Pierwsza linia wejścia zawiera wymiary pola bitwy o szerokości i wysokości oddzielone spacją.
Druga linia będzie zawierała liczbę (B) pancerników, którymi dysponuje każdy gracz.
Następnie w następnej linii typ pancernika, wymiary (szerokość i wysokość) & pozycje (współrzędna Y i współrzędna X) dla Gracza-1, a następnie dla Gracza-2 będą podane oddzielone spacją.
Następnie w następnym wierszu zostanie podana sekwencja gracza-1 (oddzielona spacją) współrzędnych lokalizacji celu (Y i X), a następnie sekwencja dla Gracza-2.
Ograniczenia:
1 < = Szerokość obszaru bitwy (M) < = 9
A < = Wysokość obszaru bitwy (N ) < = Z
1 < = Liczba pancerników < = M * N
Typ statku = {P, Q}
1 < = Szerokość pancernika < = M
A < = Wysokość pancernika < = N
1 < = współrzędna X statku < = M
A < = współrzędna Y statku < = N
Komentarze
- Możesz dodać zasady gry i komentarze przynajmniej w najważniejszych częściach kodu. Możesz także przenieść te krytyczne części kodu do podprogramów, aby kod był znacznie bardziej czytelny, chociaż wywołanie tak wielu funkcji spowolniłoby kod, ale przynajmniej stanie się znacznie bardziej czytelny dla innych, aby mogli później lepiej utrzymywać aktualizacje . Mógłbyś dodać oryginalny kod i bardziej modułowy / skomentowany kod wraz z regułami gry, być może opierając komentarze na tych zasadach.
- @ alt.126 dodał reguły i dane wejściowe. StartGame odczytuje dane wejściowe z pliku, przeprowadza walidację i tworzy BattleArea i BattleShips dla każdego gracza. BattleShips to POJO. BattleArea ma metody rozmieszczania statków i archiwizowania pocisków w oparciu o zasady. Thnx
Answer
ok, połóżmy ręce:
Nazwa zajęć dla Twojego StartGame nie jest pomocne, zmień jego nazwę na bardziej pasującą, myślę, że na przykład BattleShipGame i zamiast tego uruchom grę ze swojego kontrolera
BattleShipGame game = new BattleShipGame(); game.start(); init – metoda jest zbyt duża i nie wykonuje init , ale robi jeszcze więcej rzeczy … więc podzielmy to trochę:
- init powinien zwraca wartość logiczną (lub
Result), która wskazuje, że init się powiódł. - init wygląda jak „jest metodą delegowania , co oznacza Powinno to być bardzo mało logiczne – zamiast tego lepiej jest włożyć większość pracy w metody
- po prostu inicjować rzeczy i nie robić żadnych innych rzeczy
- użyj
Playerobiekty … - przenieś logikę gry z metody
wtedy mogłoby to wyglądać
UWAGA: metoda init może być znacznie bardziej skrócona, ale myślę, że dobrze wskazuję, co init powinno tak naprawdę do …
jak wspomniano powyżej, przeniosłeś logikę gry z metody init i umieściłeś ją w metodzie 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 zacznie się teraz w ten sposób:
public void start(){ init(); Result result = playGame(); ... //do whatever you want with your result - for example print it into the console } w przypadku Twojego BattleShip jest jeszcze kilka problemów, o których można porozmawiać. Myślę, że to był bardzo dobry pomysł, aby użyć klasy Coordinate, która na pierwszy rzut oka wygląda dobrze. Ale nie używasz go w konsekwencji. Zastanów się, jak by to było, gdybyś użył Coordinate do wysyłki zamiast int[], co spowodowałoby Twój kod również byłby łatwiejszy do odczytania, a matematyka byłaby znacznie łatwiejsza. I nie używaj char jako typu wysyłki, zamiast tego użyj wyliczenia. Ale bądźmy szczerzy, nie „nie masz” pozycji, szerokości i wysokości „, tak naprawdę masz prostokąt – więc użyj prostokąta!
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(); } ... } wymiar prostokąta (szerokość / wysokość) i ilość punktów życia można określić za pomocą typu statku
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 jest teraz znacznie łatwiejsza do użyj, zastanów się, jak łatwo możesz placeShips teraz:
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); } } } cały powyższy kod nie został skompilowany, więc mogą występować błędy w pisowni, a wiele metod nie zostało jeszcze zaimplementowanych (np. Rectangle.contains() lub inne).
podsumowanie
ale niech „spójrz na to, co mamy teraz:
- możesz łatwo zmienić typ statku bez modyfikowania dowolnego kod !!! (po prostu musisz dodać inny typ wysyłki w
ShipType) - bardzo zredukowałeś złożoność swojego kodu, nie „Nie mam niebezpiecznych obliczeń jony.
- masz oddzielne obawy, obiekty robią teraz to, co powinny.
- możesz łatwo zmienić kod na innego gracza (gra dla trzech graczy)
- możesz teraz przetestować swój kod
Komentarze
- wiem, że wciąż jest tak wiele problemów, że nie mogłem ' nie zajmij się nimi wszystkimi – ta odpowiedź była w widoku z góry, nie wchodząc w żadne szczegóły
Odpowiedź
Aby rozdzielić, musisz się upewnić, że twoje funkcje nie muszą dzwonić do siebie nawzajem, aby wykonać swoją pracę. Jeśli to możliwe, jedynymi funkcjami, które powinny wywoływać inne, są funkcje sterownika każdego podsystemu, które mają być wywoływane jak interfejs API.
Pomyśl o tym, jak musiałbyś przenosić dużo kodu tylko po to, aby dodać użyteczna funkcja, jeśli ta funkcja wywołuje inne funkcje lub używa zmiennych z innych dużych podsystemów. Jeśli podejmiesz dodatkowy wysiłek, aby zaimplementować tę funkcję w sposób, który nie zależy od niczego innego, nawet jeśli wygląda jak zduplikowany kod, będziesz mógł indywidualnie przenieść ją do innego programu, a nawet więcej, jeśli nie uzależniaj go od funkcji języka lub funkcji biblioteki, które nie są obecne w każdym kompilatorze i języku programowania, którego używasz, aby umożliwić przeniesienie dowolnego kodu, który piszesz, do dowolnego potrzebnego środowiska , to jest to nazywa się to odsprzęganiem .
Jak widać, odsprzęganie można zastosować na poziomie kompilatora, języka, środowiska systemowego, funkcji i podsystemu. Może obejmować powielanie kodu i przepisywanie w celu uzyskania samodzielnych, pozbawionych zależności procedur. Może to również oznaczać użycie bardziej znormalizowanych funkcji, aby uczynić kod bardziej przenośnym, a także może wymagać samodzielnej pracy nad implementacją lub przeniesieniem brakującej funkcjonalności do wszystkich środowisk programistycznych / systemowych, aby bez względu na system / język / kompilator użytkowania, zawsze będziesz mieć dostęp do tych samych funkcji.
O wzorcach projektowych.
Jeśli chcesz, aby kod był wysoce wielokrotnego użytku a jeśli chcesz, aby działało przez dziesięciolecia, możesz zastosować niskopoziomowe podejście do programowania złożonego procesora.
Pomyśl o zadaniu lub mikrozadaniu, które chcesz wykonać w sposób, który zawsze będzie wymagał tego samego rodzaju parametry i to zawsze zwróci wynik w dokładnie taki sam sposób.
Następnie nadaj BARDZO specyficzną nazwę tej procedurze. Będzie to kod operacji, instrukcja zaimplementowana jako funkcja / podprocedura, której możesz użyć ponownie, tak jak każdej innej natywnej instrukcji procesora. Ten sposób projektowania kodu jest stabilny i wielokrotnego użytku. Jeśli chcesz zmienić to, co zrobić, po prostu dodaj nową funkcję opcode zamiast niszczyć poprzednią prawidłową funkcjonalność.
Zastosowanie tego w całym programie jako główne podejście do projektowania może sprawić, że kod będzie jeszcze bardziej rygorystyczny. łatwiejsze do naśladowania.
Komentarze
- Dziękuję, @ alt.126. Chociaż jest to rozwiązanie uogólnione (+1), tak naprawdę liczyłem na wzorce projektowe Java / tworzenie mniejszej liczby obiektów / usuwanie nadmiarowego kodu itp.