Saper BombCounts
Mam metodę, która sprawdza wszystkie otaczające go kwadraty i zwraca liczbę bomb wokół niego. Ale to naprawdę długi i brzydki kod, więc czy można go skrócić?
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;
}
}
}
}
Odpowiedzi
Można go skrócić i „upiększyć”, refaktoryzując ograniczenia i sprawdzając je w metodzie wewnętrznej:
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;
}
Wtedy licznik min staje się trywialny (możemy założyć, że środek pola 3x3 nie ma miny, w przeciwnym razie gracz eksplodowałby i zakończył grę):
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++;
}
}
}
To, co zrobiliśmy, to rozbicie kodu na metody, z których każda implementuje małą i dobrze zdefiniowaną funkcję. Ponieważ każda metoda robi dokładnie jedną rzecz, stają się one łatwiejsze do zrozumienia, utrzymania i przetestowania.
Powinieneś sprawdzić konwencje nazewnictwa Java . Nazwy zmiennych powinny być camelCase, startingWithSmallLetter.
Nazwy zmiennych i metod powinny opisywać powód, dla którego kod istnieje. Np. mineSearchJest mylące, ponieważ zmienna nie wyszukuje min, tylko je liczy. Tak więc mineCountjest lepszą alternatywą.
Countsjest również mylący, ponieważ zawiera wartość o nazwie, MINEktóra oczywiście jest znacznikiem komórki zawierającej moje, ale zawiera również liczbę otaczających min. Zrobiłem kiedyś klon trałowca (a właściwie solwera trałowca) i użyłem tablicy zawierającej obiekty Cell. Obiekt Cell zapewnia metody sprawdzania stanu komórki (nadepnięta, oflagowana, nieznana) oraz liczby otaczających ją min, jeśli została nadepnięta.
Chociaż odpowiedź @TorbenPutkonen jest poprawna, jest to podejście proceduralne do problemu.
Nie ma nic złego w podejściach proceduralnych jako takich, ale ponieważ Java jest językiem zorientowanym obiektowo, zamiast tego moglibyśmy szukać podejść OO ...
Wyodrębniłbym sprawdzenie sąsiada do enumtakiego:
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);
}
Zaletą jest to, że to wyliczenie może żyć w swoim własnym pliku i ma bardzo ograniczoną odpowiedzialność. Oznacza to, że łatwo jest zrozumieć, co się dzieje, prawda?
W swojej metodzie obliczeniowej możesz po prostu iterować po enumstałych w następujący sposób:
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++;
}
}
}
}
W kolejnym kroku zastosowałbym zasadę „powiedz, nie pytaj”, zmieniając sygnaturę metody:
abstract int getBombValueOf(inx x, int y, boolean[] field);
wdrożenie w enumzmieniłoby się następująco:
int getBombValueOf(inx x, int y, boolean[] field){
if(0 < x && BOMB == field(x-1, y))
return 1;
else
return 0;
},
Można to uprościć do „operatora elvis”:
int getBombValueOf(inx x, int y, boolean[] field){
return (0 < x && BOMB == field(x-1, y))
? 1
: 0;
},
a użycie zmieniłoby się na:
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) );
}
}
}
Mogliśmy osiągnąć to samo (z wyjątkiem przeniesienia obliczenia sąsiada do innego pliku), używając a FunctionalInterfacei prostej kolekcji:
@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) );
}
}
}
Oczywiście moglibyśmy znacznie więcej skorzystać z zasad OO, gdyby pole gry nie było tablicą prymitywów, ale Zbiorem Obiektów . Ale to może być materiał na inną odpowiedź ...; o)