Minesweeper BombCounts
Ho un metodo che controlla tutti i quadrati circostanti e restituisce il numero di bombe intorno ad esso. Ma è un codice davvero lungo e brutto, quindi può essere abbreviato?
final int MINE =10
for (int x = 0; x < counts.length; x++) {
for (int y = 0; y < counts[0].length; y++) {
if (counts[x][y] != MINE) {
int Minesearch = 0;
if (x > 0 && y > 0 && counts[x-1][y-1] == MINE) {//up left
Minesearch++;
}
if (y > 0 && counts[x][y-1] == MINE) {//up
Minesearch++;
}
if (x < counts.length - 1 && y > 0 && counts[x+1][y-1] == MINE) {//up right
Minesearch++;
}
if (x > 0 && counts[x-1][y] == MINE) {//left
Minesearch++;
}
if (x < counts.length - 1 && counts[x+1][y] == MINE) {//right
Minesearch++;
}
if (x > 0 && y < counts[0].length - 1 && counts[x-1][y+1] == MINE) {//down left
Minesearch++;
}
if (y < counts[0].length - 1 && counts[x][y+1] == MINE) {//down
Minesearch++;
}
if (x < counts.length - 1 && y < counts[0].length - 1 && counts[x+1][y+1] == MINE) {//down right
Minesearch++;
}
counts[x][y] = Minesearch;
}
}
}
}
Risposte
Può essere accorciato e "abbellito" rifattorizzando i limiti cyhecking e il mio controllo nel metodo interno:
private boolean isWithinBounds(int x, int y) {
return x >= 0 && y >= 0 && x < width && y < height;
}
private boolean isMine(int x, int y) {
return field[x][y] == MINE;
}
Quindi il conteggio delle mine diventa banale (possiamo supporre che il centro del quadrato 3x3 non abbia una mina, altrimenti il giocatore sarebbe esploso e avrebbe concluso il gioco):
for (int x1 = x - 1; x1 <= x + 1; x1++) {
for (int y1 = y - 1; y1 <= y + 1; y1++) {
if (isWithinBounds(x1, y1) && isMine(x1, y1) {
mineCount++;
}
}
}
Quello che abbiamo fatto è stato suddividere il codice in metodi, ognuno dei quali implementa una funzione piccola e ben definita. Poiché ogni metodo fa esattamente una cosa, diventano più facili da capire, mantenere e testare.
Dovresti controllare le convenzioni di denominazione Java . I nomi delle variabili devono essere in camelCase, startingWithSmallLetter.
I nomi delle variabili e dei metodi dovrebbero descrivere il motivo per cui il codice esiste. Ad esempio, mineSearchè fonte di confusione poiché la variabile non cerca le mine, ne tiene solo il conto. Quindi mineCountè un'alternativa migliore.
Countsè anche fonte di confusione in quanto contiene un valore denominato MINEche ovviamente è un indicatore per una cella contenente la mia ma contiene anche i conteggi delle mine circostanti. Una volta ho eseguito un clone di dragamine (o in realtà un risolutore di dragamine) e ho usato un array contenente oggetti-cella. L'oggetto Cell forniva metodi per interrogare lo stato della cella (calpestata, contrassegnata, sconosciuta) e il numero di mine circostanti se fosse stata calpestata.
Sebbene la risposta di @TorbenPutkonen sia corretta, è un approccio procedurale al problema.
Non c'è niente di sbagliato negli approcci procedurali in quanto tali, ma poiché Java è un linguaggio orientato agli oggetti, potremmo invece cercare approcci OO ...
Estrarrei l'assegno del vicino in un enumsimile:
enum Direction {
NORTH{
boolean isBomb(inx x, int y, boolean[] field){
if(0 < x)
return BOMB == field(x-1, y);
else
return false;
}
},
NORTH_WEST{
boolean isBomb(inx x, int y, boolean[] field){
if(0 < x && 0 < y)
return BOMB == field(x-1, y-1);
else
return false;
}
},
SOUTH{
boolean isBomb(inx x, int y, boolean[] field){
if(field.length-1 > x)
return BOMB == field(x+1, y);
else
return false;
}
},
SOUTH_EAST{
boolean isBomb(inx x, int y, boolean[] field){
if(field.length-1 > x && field[0].length-1>y)
return BOMB == field(x+1, y+1);
else
return false;
}
}
// other directions following same pattern
abstract boolean isBomb(inx x, int y, boolean[] field);
}
Il vantaggio è che questa enumerazione potrebbe risiedere nel proprio file e ha una responsabilità molto limitata. Ciò significa che è facile capire cosa fa, non è vero?
Nel tuo metodo di calcolo puoi semplicemente iterare sulle enumcostanti in questo modo:
for (int x = 0; x < counts.length; x++) {
for (int y = 0; y < counts[0].length; y++) {
int mineCount =0;
for(Direction direction : Direction.values()) {
if (direction.isBomb(x, y, counts) ) {
mineCount++;
}
}
}
}
Come passaggio successivo, applicherei il principio "dì, non chiedere" modificando la firma del metodo:
abstract int getBombValueOf(inx x, int y, boolean[] field);
l'implementazione in enumcambierà in questo modo:
int getBombValueOf(inx x, int y, boolean[] field){
if(0 < x && BOMB == field(x-1, y))
return 1;
else
return 0;
},
Questo potrebbe essere semplificato all '"operatore elvis":
int getBombValueOf(inx x, int y, boolean[] field){
return (0 < x && BOMB == field(x-1, y))
? 1
: 0;
},
e l'utilizzo cambierebbe in questo:
for (int x = 0; x < counts.length; x++) {
for (int y = 0; y < counts[0].length; y++) {
int mineCount =0;
for(Direction direction : Direction.values()) {
mineCount +=
direction.getBombValueOf(x, y, counts) );
}
}
}
Potremmo ottenere lo stesso risultato (eccetto spostare il calcolo del vicino in un altro file) usando una FunctionalInterfacee una semplice raccolta:
@FunctionalInterface
interface Direction{
int getBombValueOf(inx x, int y, boolean[] field);
}
private final Collection<Direction> directions = new HashSet<>();
// in constructor
directions.add(new Direction() { // old style anonymous inner class
int getBombValueOf(inx x, int y, boolean[] field){
return (0 < x && BOMB == field(x-1, y))
? 1
: 0;
}
};
directions.add((x, y, field)-> { // Java8 Lambda
return (0 < x && 0 < y &&BOMB == field(x-1, y-1))
? 1
: 0;
};
// more following same pattern
// in your method
for (int x = 0; x < counts.length; x++) {
for (int y = 0; y < counts[0].length; y++) {
int mineCount =0;
for(Direction direction : directions) {
mineCount +=
direction.getBombValueOf(x, y, counts) );
}
}
}
Sicuramente potremmo trarre molto più vantaggio dai principi OO se il campo di gioco non fosse un array di primitive ma una raccolta di oggetti . Ma potrebbe essere roba per un'altra risposta ...; o)