Minesweeper BombCounts
Ich habe eine Methode, die alle umliegenden Quadrate überprüft und die Anzahl der Bomben um sie herum zurückgibt. Aber es ist wirklich langer Code und hässlich, kann es also verkürzt werden?
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;
}
}
}
}
Antworten
Es kann verkürzt und "verschönert" werden, indem die Grenzen des Cyhecking und der Minenprüfung in die interne Methode umgestaltet werden:
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;
}
Dann wird die Anzahl der Minen trivial (wir können davon ausgehen, dass die Mitte des 3x3-Quadrats keine Mine hat, sonst wäre der Spieler explodiert und hätte das Spiel beendet):
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++;
}
}
}
Was wir getan haben, ist das Aufteilen von Code in Methoden, von denen jede eine kleine und genau definierte Funktion implementiert. Da jede Methode genau eines tut, werden sie leichter zu verstehen, zu warten und zu testen.
Sie sollten die Java-Namenskonventionen überprüfen . Variablennamen sollten in camelCase, startingWithSmallLetter.
Variablen- und Methodennamen sollten den Grund beschreiben, warum der Code vorhanden ist. ZB mineSearchist verwirrend, da die Variable keine Minen durchsucht, sondern nur zählt. Somit mineCountist eine bessere Alternative.
Countsist auch verwirrend, da es einen benannten Wert enthält, MINEder offensichtlich ein Marker für eine Zelle ist, die meine enthält, aber auch die Anzahl der umgebenden Minen enthält. Ich habe einmal einen Minensuchboot-Klon (oder tatsächlich einen Minensuchboot-Löser) erstellt und ein Array mit Zellobjekten verwendet. Das Cell-Objekt stellte Methoden zum Abfragen des Status der Zelle (betreten, markiert, unbekannt) und der Anzahl der umgebenden Minen bereit, wenn sie betreten wurde.
Obwohl die Antwort von @TorbenPutkonen korrekt ist, handelt es sich um einen prozeduralen Ansatz für das Problem.
An prozeduralen Ansätzen als solchen ist nichts auszusetzen , aber da Java eine objektorientierte Sprache ist, sollten wir stattdessen nach OO-Ansätzen Ausschau halten ...
Ich würde den Nachbarscheck enumwie folgt extrahieren :
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);
}
Der Vorteil ist, dass diese Aufzählung in einer eigenen Datei gespeichert werden kann und eine sehr begrenzte Verantwortung hat. Das heißt, es ist leicht zu verstehen, was es tut, nicht wahr?
In Ihrer Berechnungsmethode können Sie die enumKonstanten einfach wie folgt durchlaufen :
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++;
}
}
}
}
Als nächsten Schritt würde ich das Prinzip "Tell, Don't Ask" anwenden, indem ich die Methodensignatur ändere:
abstract int getBombValueOf(inx x, int y, boolean[] field);
Die Implementierung in der enumwürde sich folgendermaßen ändern:
int getBombValueOf(inx x, int y, boolean[] field){
if(0 < x && BOMB == field(x-1, y))
return 1;
else
return 0;
},
Das könnte für den "elvis operator" vereinfacht werden:
int getBombValueOf(inx x, int y, boolean[] field){
return (0 < x && BOMB == field(x-1, y))
? 1
: 0;
},
und die Verwendung würde sich dahingehend ändern:
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) );
}
}
}
Wir könnten dasselbe erreichen (außer die Nachbarberechnung in eine andere Datei zu verschieben), indem wir eine FunctionalInterfaceund eine einfache Sammlung verwenden:
@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) );
}
}
}
Natürlich könnten wir viel mehr von den OO-Prinzipien profitieren, wenn das Spielfeld nicht eine Reihe von Grundelementen, sondern eine Sammlung von Objekten wäre . Aber das könnte eine andere Antwort sein ...; o)