Köszönöm a kód megosztásáért.

Ami engem kissé idegesít ezzel a kóddal, az nem használok interfészeket, absztrakt osztályokat vagy öröklést,

Az OOP végrehajtása azt jelenti, hogy bizonyos alapelveket követ (többek között):

• információ elrejtése / beágyazása
• egyetlen felelősség
• az aggodalmak szétválasztása
• KISS (Legyen egyszerű (és) hülye.)
• SZÁRAZ (Ne ismételje meg magát.)
• “Mondd! Ne kérdezz.”
• A demeter törvénye (“Ne beszélj idegenekkel!”)

Felületek, abs traktus osztályok vagy öröklési támogató kalap alapelvei, és szükség szerint alkalmazni kell őket. Nem “definiálják” az OOP-t.

IMHO a fő oka annak, hogy a megközelítése nem sikerül az OOP-ban, az, hogy a “Model” egy primitív típusú tömb char. Ez végül a játék logikájának eljárási megközelítéséhez vezet.

Ilyen felületre gondolnék:

interface GameField{ char getIcon(); Result shootAt(); } 

ahol Result felsorolás lenne:

 enum Result{ NO_HIT, PARTIAL_HIT, DESTROYED } 

És az interfésznek más megvalósításai lennének:

public class BorderField implements GameField{ private final char borderName; public BorderField(char borderName){ this.borderName = borderName; } @Override public char getIcon(){ return borderName; } @Override public Result shootAt(){ return Result.NO_HIT; } } 

public class WaterField implements GameField{ private boolean isThisFieldHit = false; @Override public char getIcon(){ return isThisFieldHit?"M": " "; } @Override public Result shootAt(){ return Result.NO_HIT; } } 

public class ShipField implements GameField{ private final Ship ship; private boolean isThisFieldHit = false; public ShipField(Ship ship){ this.ship = ship; } @Override public char getIcon(){ Result shipState = ship.getState(); switch(shipState){ case NO_HIT: return " "; case PARTIAL_HIT: return isThisFieldHit?"O":" "; case DESTROYED: return "#"; } @Override public Result shootAt(){ ship.hit(); return ship.getState(); } } 

Ennek elégnek kell lennie, remélem szerezd meg az ötletet …

Formai kérdések

Elnevezés

A jó nevek megtalálása a legnehezebb a programozásban. Tehát mindig szánjon időt arra, hogy átgondolja az azonosító nevét.

A világos oldalon kövesse a Java elnevezési szokásait.

De a metódusneveket egy igével kell kezdeni a jelen idő. Például: shipWasHit() nevét hit() kell megnevezni.
Vagy distanceBetweenPoints() be calculateDistanceBetween(). Itt a paraméterek feltárják, hogy a távolság a pontok között van, ezért nem kell ezt a módszer nevébe tenni.

Legyen részletes a változónevekben.

 double x1 = from.getX(); double y1 = from.getY(); double x2 = to.getX(); double y2 = to.getY(); 

helyett ezeket a változókat inkább így nevezték el:

 double startPointX = from.getX(); double startPointY = from.getY(); double endPointX = to.getX(); double endPointY = to.getY(); 

Vegye át nevét a problémás tartományból, ne a technikai megoldásból. pl .: SHIP_ICON csak SHIP legyen, hacsak nincs másik állandója a Ship osztályban .

Megjegyzések

A megjegyzéseknek meg kell magyarázniuk miért a kód olyan, amilyen . Távolítsa el az összes többi megjegyzést.

A megjegyzéseket csak felületen vagy absztrakt módszereken szabad használni, ha azok tartalmazzák a szerződést , amelyet a megvalósítónak teljesítenie kell.

Constants class

Rakja össze az egymáshoz tartozó dolgokat. Adja meg az állandókat abban az osztályban, amely használja őket.

Megjegyzések

• Minden érvényes pont, és nagyon hasznos. Köszönöm!

Már vannak jó válaszok, de azt gondoltam, hogy hozzáadok néhányat, ami állt amikor átnéztem a kódot.

Jelenleg az egyetlen bemeneti forrás a felhasználói beolvasás egy szkennerből. Ez meglehetősen megnehezítené, ha valamilyen számítógépes ellenfelet szeretne hozzáadni ellen játszani.

Úgy tűnik, hogy a Board osztályban van olyan kód, amely jobban megfelelhet a Player osztályban.

Konkrétan a placeShipsOnBoard () metódus tartalma.

Ez a módszer a felhasználótól veszi át a bemenetet, és létrehoz egy pozíciót. Próbáljuk meg úgy átalakítani, hogy egy játékos (ember vagy számítógép) létrehozhasson egy pozíciót.

Legyen egy interfész

public interface Player { Position placeNextShip(); void fireAt(Position target); } 

Már vannak megvalósításai egy embertől,

public class HumanPlayer implements Player { // variables @Override public Position placeNextShip(){ // uses Scanner instance to create a Position } @Override public void fireAt(Position target){ // code from your attack method } } 

És mi van alap számítógépes lejátszó

public class DumbComputer implements Player { @Override public Position placeNextShip(){ // keep choosing random locations } @Override public void fireAt(Position target){ // keep firing at random positions } } 

Ezután a fő tábla osztályban programozunk a Player felületre

while(!isShipPlacementLegal){ for(Player player : players){ // some list of players in the game // either scanner input or randomly generated Position Position shipPlacement = player.placeNextShip(); boolean validPosition = validatePos(shipPlacement); if(validPostion){ // good to go! Place ship and continue to next player } else { // prompt again, whatever else you need to do here. } } } 

Ha az összes kód egy lejátszóra (most már interfészre) utal, nem pedig konkrét megvalósításra, akkor könnyen hozzáadhatunk új típusú számítógépes lejátszókat . például. új CheatingComputer (), új HardComputer (), új MediumComputer (). Mindegyik csak másképp határozná meg, hol lőjön tovább, és hová tegye a következő hajót.

És ha ez megvan, akkor elkészíthetünk egy játékot 2 számítógépes lejátszóval, és hagyhatjuk, hogy maga is játsszon! Izgalmas jobb: D

Egy másik kapcsolódó dolog, amin változtathatunk, hogy a Game konstruktorában mindig 2 Human játékosa lesz. Megtehetjük, hogy ez egy játékoslistát vesz fel, így tetszőleges számú játékos vehet részt a játékában. Csak azért, mert az igazi csatahajó játék 2 játékosra korlátozódik, ez nem azt jelenti, hogy a tiédnek kell lennie.

Megengedhetjük az állítható rácsméretet és a tetszőleges játékosszámot. Hirtelen 100×100 rácsunk van 8 játékos mindenki számára ingyenes!

(bármelyik mennyiséget számítógéppel vezérelhetjük).

Hajóit a tábla osztályának statikus blokkjában is inicializálják. Megvan az összes igazi hajó a csatahajótól. De még egyszer, miért nem engedi meg itt a nagyobb rugalmasságot. Mi lenne, ha a hajója pontok listájából állna? Lehet, hogy van S alakú hajója, nem kellene csak a vízszintes, függőleges igazításra korlátozódnia. (Lehet, hogy ez túl van a tetején, de szerintem nagyon jó dolog ezen gondolkodni!)

Befejezek néhány apróságot, amelyek számomra viccesnek tűntek

throw new ArrayIndexOutOfBoundsException(); 

a Position osztályból. Ez nem megfelelő Kivételnek tűnik itt dobni. A Position osztályban nincs tömb, ezért ennek a Board osztály tömbjére kell utalnia. Úgy gondolom, hogy egy megfelelőbb kivétel típus az IllegalArgumentException, az ArrayIndexOutofBoundsException egy megvalósítási részlet (egy másik osztályból!). ne felejtse el megadni a megfelelő hibaüzenetet a Kivétel dobásával együtt. Pl. “az értéknek az x és y tartományon belül kell lennie”

A sor

boolean isShipHit = (ship != null) ? true : false; 

egyszerűen helyettesíthető a következővel:

boolean isShipHit = ship != null; 

Itt nincs szükség a háromszintű operátorra.

A targetHistory használata a Player osztályban, míg (targetHistory.get (point)! = null)

Itt csak egy térképet használ, hogy megnézze, van-e benne elem. Pontosan ez a készlet mert!

targetHistory = new HashSet<>(); while(targetHistory.contains(point)){ // re-prompt } 

Megjegyzések

• Nagyon köszönöm a betekintést! Mindezek a válaszok kiegészítik egymást a többi nagyon jó! Elkezdek dolgozni a 2.0 verzión mindezt szem előtt tartva.
• Semmi gond Én ‘ örülök, hogy hasznosnak találta! Sok sikert a 2.0-hoz!

Mi bosszantja nekem egy kicsit ezzel a kóddal nem használok interfészeket, absztrakt osztályokat vagy öröklődést, de itt nem találok jó használati esetet.

Ezzel nincs semmi baj a játékodban. A játékkoncepció olyan egyszerű, hogy ezekre nincs szüksége. A kis játékprogramokkal az a probléma, hogy általában nem kell nagy tervezési megoldásokat alkalmazni. Csak meg kell győződnie arról, hogy betartja-e a szokásos SZILÁRD tervezési elveket .

Most, hogy megtisztítottuk, engedjük ” nézd meg a kód néhány részletét, amelyet javítani kellene.

Az első elég nyilvánvaló. Ne írj megjegyzést a megjegyzések írása érdekében. Néhány tanár szereti arra kényszeríteni, hogy javadoc kommentet írjon mindenről. Valójában teljesen ellene vagyok, kivéve, ha valamilyen segédprogram-csomagot írok, amelyet mindenki másnak használnia kell. Általában a kódnak öndokumentálónak kell lennie. És a kódja nagyon jó munkát végez ebben. Tehát csak távolítsa el a megjegyzést alapvetően a / function / … változó jól megválasztott nevének megismétlése.

Például:

/** * Is point between boolean. * * @param point the point * @param position the position * @return the boolean */ public static boolean isPointBetween(Point point, Position position) { 

Milyen értéket ad az a megjegyzés a függvényhez?Úgy gondolom, hogy ez az egyetlen módszer, amelyet megváltoztatnék az olvashatóság érdekében. Mert nem nyilvánvaló, hogy a pozíció egy from és to pontról áll, amelyhez ellenőrizzük, hogy a point közöttük fekszik.

Mi lenne, ha a metódusnak ez az aláírása lenne:

public static boolean containsPoint(Position position, Point point) { 

Nem ” van egy kis értelme?

Itt hozzá kell tennem, hogy én általában nem vagyok a megjegyzések ellen. De egy megjegyzésnek meg kell magyaráznia, hogy MIÉRT valamit így csinálnak. Nem azt, hogy hogyan hajtják végre. Ha szeretném tudom, hogyan valósul meg, csak megnéztem a kódot.

A következő pont az, hogy az Util osztályt használom … Néhány puristától eltérően nincs semmi ellenem az Util osztály fogalma ellen. . Az Util osztályok hasznosak lehetnek hasonló funkciók összerakásához. Például java.lang.Math, amely az összes szokásos aritmetikai műveletet egy osztályba csoportosítja.

Ami engem zavar az Util osztályoddal az, hogy valójában nincs jó oka annak létezésére. Az ott található 2 funkciót csak a Board osztályon belül használják. Tehát “ugyanolyan jól lehettek private static segítő funkciók az osztály belsejében.

Valójában még egy kicsit jobban is tehetünk, miután megváltoztattuk az aláírást arra, amire Javasoltam korábban. Mi lenne, ha containsPoint(Position position, Point point) { -t tennénk az Position osztályba, ahelyett, hogy a Position a metódus paramétereként? Akkor használhatjuk így:

Position shipPosition = //some ship"s position if(shipPosition.contains(targetPoint)) { //handle ship hit } 

Nagyon jól illik oda, nem igaz?

Ha a Positions osztályról beszélünk. Furcsa érzésem volt ezzel kapcsolatban, miközben végignéztem a kódját. Először azt hittem, hogy “something[][] nincs a tábla képviselete. Azt hittem, hogy pontokként képviselted az egész kódbázist. Ez működhet, de a tábla nyomtatását kínossá teszi. És akkor észrevettem, hogy van char[][] az Board osztályban. De akkor nem lenne ésszerűbb a hajókat azonnal belemenni ez a rács anélkül, hogy közbenső Position osztálya lenne?

Egy másik veszélyes dolgot is észrevettem a placeShipsOnBoard() kapcsán. Tényleg csak arra bízná a felhasználót, hogy 2 érvényes koordinátát adjon meg? Mi van, ha a felhasználó vicces próbál lenni, és = (1,1) – = (2,2) értéket ad meg? Engedjük meg ezt? Vagy mi van, ha azt akarja, hogy egy 5 hosszúságú hajót adjon meg, és ő = (1,1) -től = (1,1) -ig írja be, lényegében egyetlen négyzetre zsugorítva a hajót (amit ötször kell eltalálnia! Mivel a hajó 5 élete van). Nem szabad megakadályoznunk abban, hogy így csaljon?

Nézzük meg a hajók elhelyezésének alternatív módját. Először hagyja, hogy a felhasználó döntse el, hogy vízszintesen vagy függőlegesen szeretné-e elhelyezni a hajót. Ezután adja meg a hajó felső / bal koordinátáját. Mi magunk fogjuk kiszámolni a fennmaradó pontokat.

Itt nézhet ki a módszer tényleges megvalósítása:

private Scanner scanner = new Scanner(System.in); private void placeShipsOnBoard() { System.out.printf("%nAlright - Time to place out your ships%n%n"); for(Ship ship : ships) { //awesome for-each loop is better here boolean horizontal = askValidShipDirection(); Point startingPoint = askValidStartingPoint(ship, horizontal); placeValidShip(ship, startingPoint, boolean horizontal); } } private boolean askValidShipDirection() { do { System.out.println("Do you want to place the ship horizontally (H) or vertically (V)?"); String direction = Scanner.nextLine().trim(); while ( !"H".equals(direction) && !"V".equals(direction); return "H".equals(direction); //note here: use "constant".equals(variable) so nullpointer is impossible. //probably not needed, but it"s best practice in general. } private Point askValidStartingPoint(Ship ship, boolean horizontal) { do { //note: do-while more useful here System.out.printf("%nEnter position of %s (length %d): ", ship.getName(), ship.getSize()); Point from = new Point(scanner.nextInt(), scanner.nextInt()); while(!isValidStartingPoint(from, ship.getLength(), horizontal)); return from; } private boolean isValidStartingPoint(Point from, int length, boolean horizontal) { int xDiff = 0; int yDiff = 0; if(horizontal) { xDiff = 1; } else { yDiff = 1; } for(int i = 0; i < lenth; i++) { if(!isInsideBoard(i,from.getY()) { return false; } if(!Constants.BOARD_ICON.equals(board[from.getY()+i*yDiff][from.getX()+i*xDiff])){ return false; } } return true; } private boolean isInsideBoard(int x, int y){ return x <= Constants.BOARD_SIZE && x >= 0 && y <= Constants.BOARD_SIZE && y >= 0 && x <= Constants.BOARD_SIZE && x >= 0 && y <= Constants.BOARD_SIZE && y >= 0; } private void placeValidShip(Ship ship, Point startingPoint, boolean horizontal) { int xDiff = 0; int yDiff = 0; if(horizontal) { xDiff = 1; } else { yDiff = 1; } for(int i = 0; i < ship.getLenth() ; i++) { board[ship.getY() + i*yDiff][ship.getX()+i*xDiff] = Constants.SHIP_ICON; } } 

Most, hogy éppen Ha a hajókat közvetlenül a fedélzetre helyezzük, eltávolíthatjuk a Position osztályt és az összes hivatkozást. Ez azt jelenti, hogy már nem tudjuk, hogy egy hajó elsüllyedt-e vagy sem.

Miközben ezt nyikkantam, észrevettem, hogy @Timothy Truckle is küldött már választ. Nagyon szeretem azt a megoldását, hogy dedikált Field s-eket használ a tábla képviseletére char “helyett. a hely hajó módszerünk a következőre változik:

private void placeValidShip(Ship ship, Point startingPoint, boolean horizontal) { int xDiff = 0; int yDiff = 0; if(horizontal) { xDiff = 1; } else { yDiff = 1; } for(int i = 0; i < ship.getLenth() ; i++) { board[ship.getY() + i*yDiff][ship.getX()+i*xDiff] = new ShipField(ship); } } 

Így ellenőrizhetjük, hogy egy hajó teljesen megsemmisült-e, vagy csak részlegesen eltalálta-e, ha megtámadtunk egy Field.

Ahelyett, hogy folytatnám ezt a választ, azt javaslom, hogy olvassa el a @Timothy-t is, és nézze meg mindkét válaszunk jó pontjait. Első ránézésre vagy megegyezünk, vagy egyszerűen kiegészítjük egymás válaszait. Szüksége lenne tehát valamilyen tisztességes útmutatásra a kód javításával kapcsolatban. >

Más válaszok szinte mindenre rámutattak, ezért csak egy dolgot teszek hozzá. Úgy néz ki nekem, hogy kivételeket használsz valamilyen módon az áramlásszabályozáshoz. A kivételek nem a vezérlési folyamat mechanizmusai .

public Position(Point from, Point to) { if (from.getX() > Constants.BOARD_SIZE || from.getX() < 0 || from.getY() > Constants.BOARD_SIZE || from.getY() < 0 || to.getX() > Constants.BOARD_SIZE || to.getX() < 0 || to.getY() > Constants.BOARD_SIZE || to.getY() < 0) { throw new ArrayIndexOutOfBoundsException(); } this.from = from; this.to = to; } 

És akkor

} catch (IndexOutOfBoundsException e) { System.out.println("Invalid coordinates - Outside board"); } 

Szerintem ellenőriznie kell, hogy a megadott koordináták a táblán belül vannak-e, mielőtt megpróbálná létrehozni a Position -t. Ez felhasználói bemenet és teljesen ésszerű megtenni. Már ellenőrzi azt a ship.getSize() != Utils.distanceBetweenPoints(from, to) -t. Ezt még maga a Board objektumban is megtenné, ahelyett, hogy a Constants.BOARD_SIZE.