Сапер Бомба

Aug 28 2020

У меня есть метод, который проверяет все окружающие его квадраты и возвращает количество бомб вокруг них. Но это действительно длинный и уродливый код, можно ли его сократить?

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

Ответы

12 TorbenPutkonen Aug 28 2020 at 12:28

Его можно сократить и "украсить" путем рефакторинга проверки границ и проверки майнинга во внутренний метод:

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

Тогда подсчет мин становится тривиальным (можно предположить, что в центре квадрата 3x3 нет мин, иначе игрок взорвался бы и закончил игру):

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

Что мы сделали, так это разбили код на методы, каждый из которых реализует небольшую и четко определенную функцию. Поскольку каждый метод выполняет одну задачу, их становится легче понять, поддерживать и тестировать.

Вы должны проверить соглашения об именах Java . Имена переменных должны быть camelCase, startingWithSmallLetter.

Имена переменных и методов должны описывать причину, по которой существует код. Например mineSearch, сбивает с толку, поскольку переменная не ищет шахты, а просто ведет их подсчет. Таким образом mineCount, это лучшая альтернатива.

Countsтакже сбивает с толку, поскольку содержит значение с именем, MINEкоторое, очевидно, является маркером ячейки, содержащей шахту, но также содержит количество окружающих ее мин. Однажды я сделал клон тральщика (или на самом деле решатель тральщика) и использовал массив, содержащий объекты Cell. Объект Cell предоставил методы для запроса статуса ячейки (наступил, помечен, неизвестен) и количества окружающих мин, если на нее наступили.

1 TimothyTruckle Aug 29 2020 at 20:29

Хотя ответ @TorbenPutkonen правильный, это процедурный подход к проблеме.

В процедурных подходах как таковых нет ничего плохого , но поскольку Java - объектно-ориентированный язык, мы могли бы вместо этого искать объектно-ориентированные подходы ...

Я бы извлек проверку соседей enumследующим образом:

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

Преимущество в том, что это перечисление может находиться в собственном файле и имеет очень ограниченную ответственность. Значит, легко понять, что это такое, не так ли?

В своем методе расчета вы можете просто перебирать enumконстанты следующим образом:

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

В качестве следующего шага я бы применил принцип «говори, не спрашивай», изменив сигнатуру метода:

abstract int getBombValueOf(inx x, int y, boolean[] field);

реализация в enumфайле изменится следующим образом:

     int getBombValueOf(inx x, int y, boolean[] field){
       if(0 < x && BOMB == field(x-1, y))
         return 1;
       else
         return 0;
     },

Это можно упростить до "оператора элвиса":

     int getBombValueOf(inx x, int y, boolean[] field){
       return (0 < x && BOMB == field(x-1, y))
         ? 1 
         : 0;
     },

и использование изменится на это:

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

Мы могли бы добиться того же (за исключением перемещения вычисления соседей в другой файл), используя FunctionalInterfaceпростой набор и:

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

Конечно, мы могли бы извлечь гораздо больше пользы из принципов объектно-ориентированного программирования, если бы игровое поле было не массивом примитивов, а коллекцией объектов . Но это может быть материалом для другого ответа ...; o)