Kiitos koodin jakamisesta.

Mikä häiritsee minua hieman tällä koodilla, en käytä käyttöliittymiä, abstrakteja luokkia tai perintöä,

OOP: n tekeminen tarkoittaa, että noudatat tiettyjä periaatteita, jotka ovat (muun muassa):

• tietojen piilottaminen / kapseloiminen
• yksittäinen vastuu
• huolenaiheiden erottaminen
• KISS (pidä yksinkertaisena (ja) tyhmänä.)
• KUIVA (Älä toista itseäsi.)
• ”Kerro! Älä kysy”.
• Demeter-laki (”Älä puhu muukalaisten kanssa!”)

Liitännät, abs traktaatioluokissa tai perintötukihatoissa ja niitä tulisi käyttää tarvittaessa. He eivät ”määrittele” OOP: ta.

IMHO on tärkein syy siihen, miksi lähestymistapa epäonnistuu OOP: ssa, että ”Malli” on primitiivisen tyyppinen taulukko char. Tämä johtaa lopulta pelilogiikan menettelytapaan.

Ajattelen tällaista käyttöliittymää:

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

missä Result olisi luettelo:

 enum Result{ NO_HIT, PARTIAL_HIT, DESTROYED } 

Ja minulla olisi erilaisia käyttöliittymän toteutuksia:

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(); } } 

Tämän pitäisi riittää, toivottavasti saada idea …

Muodolliset kysymykset

Nimeäminen

Hyvien nimien löytäminen on vaikeinta ohjelmoinnissa. Ota siis aina aikaa ajatellaksesi tunnisteiden nimiäsi.

Noudatat valoisalla puolella Java-nimityskäytäntöjä.

Mutta sinun pitäisi saada metodinimesi alkamaan verbillä nykyinen aika.Eg: shipWasHit() pitäisi olla nimeltään hit().
Tai distanceBetweenPoints() pitäisi olla olla calculateDistanceBetween(). Tässä parametrit paljastavat, että etäisyys on pisteiden välillä, joten sitä ei tarvitse lisätä menetelmän nimeen.

Ole tarkkoja muuttujien nimissäsi.

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

sijaan näiden muuttujien tulisi olla nimetty näin:

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

Ota nimesi ongelma-alueelta, älä teknisestä ratkaisusta. esim .: SHIP_ICON tulee olla vain SHIP, ellet ole toista vakiota luokassa Ship .

Kommentit

Kommenttien tulisi selittää miksi koodi on kuin se on . Poista kaikki muut kommentit.

Kommentteja tulisi käyttää vain käyttöliittymässä tai abstrakteissa menetelmissä, jos ne sisältävät sopimuksen , jonka toteuttajan on täytettävä.

Vakioluokka

Kokoa yhteen kuuluvat asiat. Määritä vakiot niitä käyttävälle luokalle.

Kommentit

• Kaikki kelvolliset kohdat ja todella hyödyllisiä. Kiitos!

Hyviä vastauksia on jo olemassa, mutta ajattelin lisätä joitain asioita, jotka seisoivat minulle, kun katselin koodia.

Tällä hetkellä ainoa syötteen lähde on käyttäjän syöttö skannerista. Tämä tekisi melko vaikeaksi, jos haluat lisätä jonkinlaisen tietokoneen vastustajan pelaa vastaan.

Näyttää siltä, että Board-luokassa on joitain koodeja, jotka sopivat paremmin Player-luokkaan.

Erityisesti placeShipsOnBoard () -menetelmän sisältö.

Tämä menetelmä ottaa käyttäjän panoksen ja luo sijainnin. Yritetään uudelleen rakentaa tämä niin, että pelaaja (ihminen tai tietokone) voi luoda sijainnin.

Annetaan käyttöliittymä

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

Meillä on jo toteutuksia ihmiseltä,

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 } } 

Ja entä perustietokoneen soitin

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

Sitten päätaululuokassa ohjelmoimme soittimen käyttöliittymään

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. } } } 

Jos kaikki koodit viittaavat Playeriin (nyt käyttöliittymään) eikä konkreettiseen toteutukseen, voimme helposti lisätä uudentyyppisiä tietokonesoittimia . esimerkiksi. uusi CheatingComputer (), uusi HardComputer (), uusi MediumComputer (). Jokainen vain päättäisi, mihin ampua seuraavaksi ja minne seuraava alus eri tavalla.

Ja jos meillä olisi tämä, voisimme tehdä pelin kahdella tietokoneella ja antaa sen pelata itse! Jännittävä oikeus: D

Toinen asiaan liittyvä asia, jota voimme muuttaa, on se, että Game-konstruktorissasi on aina 2 Human-pelaajaa. Voisimme tehdä tämän ottamaan luettelon pelaajista, joten sinulla voi olla mikä tahansa määrä pelaajia pelissäsi. Pelkästään siksi, että todellinen taistelulaiva on rajoitettu kahteen pelaajaan, ei tarkoita, että sinun täytyy olla.

Voimme sallia säädettävän ruudukon koon ja mielivaltaisen pelaajanumeron. Yhtäkkiä meillä on 100×100 ruudukko, jossa on 8 soitin kaikille ilmaiseksi!

(mikä tahansa määrä voidaan ohjata tietokoneella).

Aluksesi alustetaan myös laudan luokan staattisessa lohkossa. Sinulla on kaikki todelliset alukset taistelulaivalta. Mutta jälleen kerran, miksi ei sallita enemmän joustavuutta täällä. Entä jos alus koostuisi luettelosta pisteistä? Sinulla voisi olla S-muotoisia aluksia, niiden ei tarvitse rajoittua vaakasuoraan pystysuoraan. (Tämä voi olla ylhäällä, mutta mielestäni se on hienoa ajatella!)

Viimeistelen muutama pieni asia, joka näytti minusta hauskalta

throw new ArrayIndexOutOfBoundsException(); 

Sijainti-luokassa. Tämä tuntuu sopimattomalta poikkeukselta heittää tänne. Position-luokassa ei ole taulukkoa, joten tämän on viitattava Board-luokan matriisiin. Luulen, että sopivampi poikkeustyyppi olisi IllegalArgumentException, ArrayIndexOutofBoundsException on toteutuksen yksityiskohta (eri luokasta!). muista myös antaa asianmukainen virhesanoma yhdessä poikkeuksen heittämisen kanssa. esim. ”arvon on oltava alueella x ja y”.

Rivi

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

voidaan yksinkertaisesti korvata sanalla

boolean isShipHit = ship != null; 

Ternary-operaattoria ei tarvita.

targetHistoryn käyttö Player-luokassa, kun taas (targetHistory.get (point)! = null)

Tässä käytät karttaa ainoana tarkoituksena nähdä, onko elementti siinä. Juuri tämä on sarja puolesta!

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

Kommentit

• Kiitos paljon oivalluksista! Kaikki nämä vastaukset täydentävät kutakin muut todella hyvin! Alan työskennellä version 2.0 kanssa kaikki tämä mielessä.
• Ei hätää, olen ’ iloinen, että pidit siitä hyödyllisenä! Onnea 2.0: n kanssa!

Mikä ärsyttää minua vähän tällä koodilla, enkä käytä käyttöliittymiä, abstrakteja luokkia tai perintöä, mutta en löytänyt heille hyvää käyttötapaa täältä.

Tässä ei ole mitään vikaa pelissä. Pelikonsepti on niin yksinkertainen, että et tarvitse mitään näistä. Pienien leluohjelmien ongelma on, että sinun ei yleensä tarvitse soveltaa suuria suunnitteluratkaisuja. Sinun tarvitsee vain varmistaa, että noudatat tavallisia SOLID-suunnitteluperiaatteita .

Nyt kun olemme saaneet selvityksen, anna ”s katso joitain koodisi yksityiskohtia, joita tulisi parantaa.

Ensimmäinen on melko ilmeinen. Älä kirjoita kommentteja kommenttien kirjoittamisen vuoksi. Jotkut opettajat haluavat pakottaa sinut kirjoittamaan javadoc-kommentin kaikkeen. Vastustan tätä täysin, paitsi kirjoittaessani jonkinlaista apuohjelmapakettia, jota kaikkien muiden on käytettävä. Normaalisti koodin tulisi olla itse dokumentoitava. Ja koodisi tekee siinä todella hyvää työtä. Poista siis kommentti toistetaan periaatteessa muuttujan / function / … hyvin valittu nimi.

Esimerkiksi:

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

Mitä arvoa kommentti lisää toimintoon?Luulen, että tämä on myös ainoa menetelmä, jota muutan tekemään siitä helpommin luettavissa. Koska ei ole ilmeistä, että sijainti koostuu from – ja to -kohdasta, jolle tarkistamme, onko point on niiden välissä.

Entä jos menetelmällä olisi tämä allekirjoitus:

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

Eikä Onko sinulla hieman järkevämpää?

Haluan lisätä tähän, että en kuitenkaan vastusta kommentteja yleensä. Mutta kommentin tulisi selittää MIKSI jotain tehdään näin. Ei miten se toteutetaan. Jos haluan tiedän, miten se toteutetaan, olen vain tarkastellut koodia.

Seuraava kohta on kyseisen Util-luokan käyttäminen … Toisin kuin jotkut puristit, minulla ei ole mitään Util-luokkien käsitettä vastaan . Util-luokista voi olla hyötyä samankaltaisten toimintojen yhdistämisessä. Esimerkiksi java.lang.Math, joka ryhmittelee kaikki tavanomaiset aritmeettiset operaatiot yhteen luokkaan.

Minua häiritsevä asia Util-luokan kanssa on, että sen olemassaololle ei ole oikeaa syytä. Kahta toimintoa, joita sinulla on siellä, käytetään koskaan vain Board-luokassa. Joten he voisivat ”olla yhtä hyvin private static auttajatoimintoja kyseisen luokan sisällä.

Itse asiassa voimme jopa tehdä hieman paremmin muutettuamme allekirjoituksen mihin Ehdotin aiemmin. Entä jos asetamme containsPoint(Position position, Point point) { luokan Position sisälle sen sijaan, että meillä olisi Position menetelmän parametrina? Sitten voimme käyttää sitä seuraavasti:

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

Se sopii todella hyvin sinne, eikö niin?

Puhuminen Positions -luokasta. Minulla oli outo tunne tästä, kun katselin koodiasi. Aluksi luulin, että sinulla ei ole something[][] edustamaan taulua. Luulin, että olit edustanut kaikkea pisteinä koko koodipohjassa. Tämä voisi toimia, mutta tekee levyn tulostamisesta hankalaa. Ja sitten huomasin, että char[][] Board -luokan sisällä on. Mutta silloin ei olisi järkevämpää laittaa aluksia heti että ruudukko ilman keskitason sijaintiluokkaa?

Huomasin myös toisen vaarallisen asian placeShipsOnBoard(): stä. Pitäisikö sinun vain luottaa siihen, että käyttäjä antaa kaksi kelvollista koordinaattia? Entä jos käyttäjä yrittää olla hauska ja syöttää arvot = (1,1) – = (2,2)? Pitäisikö meidän sallia tämä? Tai mitä jos haluat hänen syöttävän aluksen, jonka pituus on 5, ja hän syöttää arvosta = (1,1) – = (1,1), kutistuu laivan olennaisesti yhdeksi neliöksi (että sinun täytyy lyödä viisi kertaa! Koska alus on 5 ihmistä). Eikö meidän pitäisi estää häntä huijaamasta näin?

Katsotaanpa vaihtoehtoista tapaa käsitellä alusten sijoittamista. Anna käyttäjän ensin valita, haluatko sijoittaa aluksen vaaka- tai pystysuoraan. Anna hänen sitten syöttää aluksen ylin / vasen koordinaatti. Laskemme loput pisteet itse.

Menetelmän todellinen toteutus voi näyttää tältä:

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; } } 

Nyt kun juuri sijoittamalla alukset suoraan aluksella voimme poistaa Position -luokan ja kaikki sen viitteet. Tämä tarkoittaa sitä, että emme enää tiedä onko alus uponnut vai ei.

Nipistämällä huomasin, että myös @Timothy Truckle lähetti vastauksen. Rakastan todella hänen ratkaisuaan käyttää omistautuneita Field ia char ”: iden sijasta taulua edustamaan.

niin paikkalaivamenetelmämme muuttuu:

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); } } 

Näin voimme tarkistaa, tuhoutuuko alus kokonaan vai osuuko se osittain, kun hyökkäämme Field.

Sen sijaan, että jatkaisit tätä vastausta, suosittelen, että luet myös @Timothy” -sivut ja tarkastelet molempien vastausten hyviä puolia. Ensi silmäyksellä olemme joko yhtä mieltä tai yksinkertaisesti täydentävät toistenne vastausta. Joten sinulla pitäisi olla kunnollisia vinkkejä koodisi parantamiseen 🙂

Kommentit

• Paljon kiitoksia yksityiskohtaisesta katsauksesta! Te kaikki sanotte vakaasti. Koska kaikki vastaukset olivat niin hyviä, annan ’ vastauksen henkilölle, joka vastasi ensin.
• ahem ” Tässä ei ole mitään vikaa pelissäsi. Pelikonsepti on niin yksinkertainen, että et ’ t tarvitse mitään näistä. ” – Minun on oltava eri mieltä: Se on ’ parempi harjoitella kaikki ensin helpossa tilassa. Jos et voi ’ käyttää käsitteitä ja malleja jollekin yksinkertaiselle, voit varmasti ’ t levitä niitä johonkin monimutkaisempaan.
• Määritelmiä koskeva osa on erityisen tärkeä. Tarpeettomat kommentit ovat täydellinen ajan ja energian tuhlaaminen ylläpitää ja tarkistaa. Kommentoi vain, jos et saa koodia ja toimintoja selittääkseen itseään.

Muut vastaukset ovat osoittaneet melkein kaiken, joten lisätään vain yksi asia. Näyttää siltä minulle, että käytät poikkeuksia jotenkin suorittaaksesi virtauksen hallintaa. Poikkeukset eivät ole ohjausvirtausmekanismeja .

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; } 

Ja sitten

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

Mielestäni sinun on vahvistettava, että annetut koordinaatit ovat piirilevyssä, ennen kuin yrität luoda Position. Tämä on käyttäjän panos ja on täysin kohtuullista tehdä se. Olet jo vahvistamassa kyseisen ship.getSize() != Utils.distanceBetweenPoints(from, to). Voit jopa tehdä sen itse objektissa Board sen sijaan, että sinulla olisi sijainnin tarkistus kohteelle Constants.BOARD_SIZE.