Существуют ли методы разделения метода с помощью аргумента флага? - PullRequest
8 голосов
/ 24 ноября 2010

У меня есть метод с аргументом флага. Я думаю, что передача логического значения в метод является плохой практикой (усложняет сигнатуру, нарушает принцип «каждый метод делает одно») Я думаю, что лучше разделить метод на два разных метода. Но если я сделаю это, оба метода будут очень похожи (дублирование кода).

Интересно, есть ли какие-то общие методы для разделения методов с аргументом флага на два отдельных метода.

Вот код моего метода (Java):

int calculateNumOfLiveOrDeadNeighbors(Cell c, int gen, boolean countLiveOnes) {
   int x = c.getX();
   int y = c.getY();
   CellState state;
   int aliveCounter = 0;
   int deadCounter = 0;
   for (int i = x - 1; i <= x + 1; i++) {
      for (int j = y - 1; j <= y + 1; j++) {
         if (i == x && j == y)
            continue;
         state = getCell(i, j).getCellState(gen);
         if (state == CellState.LIVE || state == CellState.SICK){
            aliveCounter++;
         }
         if(state == CellState.DEAD || state == CellState.DEAD4GOOD){
            deadCounter++;
         }
      }
   }
   if(countLiveOnes){
      return aliveCounter;
   }
   return deadCounter;
}

Ответы [ 9 ]

5 голосов
/ 24 ноября 2010

Если вам не нравится логическое значение в вашей подписи, вы можете добавить два разных метода без него, рефакторинг в private основной:

int calculateNumOfLiveNeighbors(Cell c, int gen) {
  return calculateNumOfLiveOrDeadNeighbors(c, gen, true);
}
int calculateNumOfDeadNeighbors(Cell c, int gen) {
  return calculateNumOfLiveOrDeadNeighbors(c, gen, false);
}

ИЛИ

вы можете закодировать Result Class или int array в качестве выходного параметра для хранения обоих результатов;это позволит вам избавиться от раздражающего логического параметра.

4 голосов
/ 24 ноября 2010

Полагаю, это зависит от каждого отдельного случая.

В этом примере, на мой взгляд, у вас есть два варианта.

Скажем, вы хотите разделить вызов calculateNumOfLiveOrDeadNeighbors()

в два:

calculateNumOfLiveNeighbors() 

и

calculateNumOfDeadNeighbors()

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

private int countCells(Cell c, int gen, Filter filter)
{
    int x = c.getX();
    int y = c.getY();
    CellState state;
    int counter = 0;
    for (int i = x - 1; i <= x + 1; i++) 
    {
        for (int j = y - 1; j <= y + 1; j++) 
        {
            if (i == x && j == y)
                continue;
            state = getCell(i, j).getCellState(gen);
            if (filter.countMeIn(state))
            {
                counter++;
            }
        }
    }
    return counter;
 }

 private interface Filter
 {
      boolean countMeIn(State state);
 }

 public int calculateNumOfDeadNeighbors(Cell c, int gen)
 {
     return countCells(c, gen, new Filter()
                       { 
                           public boolean countMeIn(CellState  state)
                           {
                              return (state == CellState.DEAD || state == CellState.DEAD4GOOD);
                           }
                        });
  }

 public int calculateNumOfLiveNeighbors(Cell c, int gen)
 {
     return countCells(c, gen, new Filter()
                       { 
                           public boolean countMeIn(CellState  state)
                           {
                              return (state == CellState.LIVE || state == CellState.SICK);
                           }
                        });
  }

Это громоздко, может быть, даже не стоит боли. В качестве альтернативы вы можете использовать монаду для хранения результатов вашего статистического расчета, а затем использовать getDeadCounter() или getLiveCounter() для монады, как уже предлагали многие.

4 голосов
/ 24 ноября 2010
  • вы можете попытаться извлечь общие функции одним способом и использовать только определенные функции
  • вы можете создать закрытый метод с этим флагом и вызывать его из двух открытых методов. Таким образом, ваш общедоступный API не будет иметь подписи «сложного» метода, и у вас не будет дублированного кода
  • создайте метод, который возвращает оба значения, и выберите одно в каждом вызывающем объекте (открытый метод).

В приведенном выше примере я думаю, что 2-й и 3-й варианты более применимы.

1 голос
/ 24 ноября 2010

Я был бы склонен оставить карту из перечисления CellState для подсчета, затем добавить LIVE и SICK или DEAD и DEAD4GOOD по мере необходимости.

int calculateNumOfLiveOrDeadNeighbors(Cell c, int gen, boolean countLiveOnes) {
    final int x = c.getX();
    final int y = c.getY();
    final HashMap<CellState, Integer> counts = new HashMap<CellState, Integer>();
    for (CellState state : CellState.values())
        counts.put(state, 0);

    for (int i = x - 1; i < x + 2; i++) {
        for (int j = y - 1; j < y + 2; j++) {
            if (i == x && j == y)
                continue;
            CellState state = getCell(i, j).getCellState(gen);
            counts.put(state, counts.get(state) + 1);
        }
    }
    if (countLiveOnes)
        return counts.get(CellState.LIVE) + counts.get(CellState.SICK);
    else
        return counts.get(CellState.DEAD) + counts.get(CellState.DEAD4GOOD);
}
1 голос
/ 24 ноября 2010

С точки зрения использования рефакторинга, некоторые вещи, которые вы можете сделать:

  • скопируйте метод и создайте две версии, одну с истинным жестким кодом, а другую с ложным жестким кодом. Ваши инструменты рефакторинга должны помочь вам встроить эту константу и при необходимости удалить код.
  • воссоздайте метод, который вызывает правильный метод true / false, как указано выше, для обратной совместимости. Затем вы можете встроить этот метод.
1 голос
/ 24 ноября 2010

Как сказал Божо: «Но комбинируйте точки 2 и 3 по-другому:

»

Создайте (возможный закрытый метод), который возвращает оба (живой и мертвый) и (только если в большинстве случаев вам нужен мертвый или живой отдельный), затем добавьте два метода, которые выбирают мертвых или оба из результата:

DeadLiveCounter calcLiveAndDead(..) {}
int calcLive(..) { return calcLiveAndDead(..).getLive; }
int calcDead(..) { return calcLiveAndDead(..).getDead; }
1 голос
/ 24 ноября 2010

ИМО, этот так называемый принцип "каждый метод делает одно" должен применяться избирательно.Ваш пример один, где, вероятно, лучше НЕ применять его.Скорее, я бы немного упростил реализацию метода:

int countNeighbors(Cell c, int gen, boolean countLive) {
   int x = c.getX();
   int y = c.getY();
   int counter = 0;
   for (int i = x - 1; i <= x + 1; i++) {
      for (int j = y - 1; j <= y + 1; j++) {
         if (i == x && j == y)
            continue;
         CellState s = getCell(i, j).getCellState(gen);
         if ((countLive && (s == CellState.LIVE || s == CellState.SICK)) ||
             (!countLive && (s == CellState.DEAD || s == CellState.DEAD4GOOD))) {
            counter++;
         }
      }
   }
   return counter;
}
1 голос
/ 24 ноября 2010

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

0 голосов
/ 24 ноября 2010

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

...