2010-11-24 34 views
8

J'ai une méthode avec un argument flag. Je pense que passer un booléen à une méthode est une mauvaise pratique (complique la signature, viole le principe «chaque méthode fait une chose»). Je pense que diviser la méthode en deux méthodes différentes est préférable. Mais si je fais cela, les deux méthodes seraient très similaires (duplication de code).Existe-t-il des techniques pour diviser une méthode avec un argument flag?

Je me demande s'il existe des techniques générales pour diviser des méthodes avec un argument d'indicateur en deux méthodes distinctes.

est ici le code de ma méthode (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; 
} 

Répondre

3

Je suppose que cela dépend de chaque cas .

Dans cet exemple, vous avez deux choix, à mon avis.

que vous voulez diviser l'appel calculateNumOfLiveOrDeadNeighbors()

en deux:

calculateNumOfLiveNeighbors() 

et

calculateNumOfDeadNeighbors() 

Vous pouvez utiliser Template Method pour déplacer la boucle à une autre méthode. Vous pouvez l'utiliser pour compter les cellules mortes/vivantes dans les deux méthodes.

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

C'est lourd, peut-être même pas la douleur. Vous pouvez, alternativement, utiliser un monad pour stocker les résultats de votre calcul de statistiques et ensuite utiliser getDeadCounter() ou getLiveCounter() sur la monade, comme beaucoup l'ont déjà suggéré.

+0

Bonne idée, mais utilisez des classes internes statiques pour implémenter le filtre. – Ralph

4
  • vous pouvez essayer d'extraire les fonctionnalités communes dans une seule méthode et utiliser uniquement les fonctionnalités spécifiques
  • vous pouvez créer une méthode privée avec ce drapeau, et l'invoquer des deux méthodes publiques. Ainsi votre API publique n'aura pas la signature de la méthode 'compliquée', et vous n'aurez pas de code dupliqué
  • faites une méthode qui retourne les deux valeurs, et choisissez-en une dans chaque appelant (méthode publique).

Dans l'exemple ci-dessus, je pense que les 2ème et 3ème options sont plus applicables.

1

On dirait que l'approche la plus sémantiquement propre consisterait à retourner un objet résultat qui contient les deux valeurs, et à laisser le code appelant extraire ce dont il se soucie de l'objet résultat.

+0

+1 pour l'optimisation. Sinon, la méthode devrait être appelée deux fois pour obtenir des voisins morts et vivants. – khachik

5

Si vous n'aimez pas les booléen sur votre signature, vous pouvez ajouter deux méthodes différentes sans elle, refactoring à private le principal:

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

OU

vous pourriez coder un Classe de résultats ou int tableau comme paramètre de sortie pour stocker les deux résultats; cela vous permettrait de vous débarrasser du paramètre booléen ennuyeux.

+1

Je préférerais la méthode privée avec des méthodes déléguées publiques plutôt que toute autre suggestion. – heikkim

1

IMO, ce soi-disant "chaque méthode fait une chose" principe doit être appliqué sélectivement. Votre exemple est celui où, il est probablement préférable de ne pas l'appliquer. Au contraire, je venais de simplifier la mise en œuvre de la méthode un peu:

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

Comme Bozho dit: Mais, mais combinerai point 2 et 3 de l'autre façon arround:

Créer une (possible méthode privée) qui retourne les deux (vivants et morts) et (seulement si vous avez besoin séparé mort ou vivant dans la plupart des cas), puis ajouter deux méthodes pICK morts, ou les deux sur le résultat:

DeadLiveCounter calcLiveAndDead(..) {} 
int calcLive(..) { return calcLiveAndDead(..).getLive; } 
int calcDead(..) { return calcLiveAndDead(..).getDead; } 
1

En termes d'utilisation de refactoring, certaines choses que vous pouvez faire sont;

  • Copiez la méthode et créez deux versions, l'une avec le vrai codage en dur et l'autre avec le codage en dur. Vos outils de refactoring devraient vous aider à aligner cette constante et à supprimer le code si nécessaire.
  • recréer la méthode qui appelle la bonne méthode true/false comme ci-dessus pour la rétrocompatibilité. Vous pouvez ensuite intégrer cette méthode.
0

avoir une méthode privée qui est une copie exacte et pâte de ce que vous avez actuellement. Puis créer deux nouvelles méthodes, chacune avec un nom plus descriptif qui appellent simplement votre méthode privée avec booléen approprié

1

Je serais enclin ici pour garder une carte de l'énumération CellState pour compter, puis ajouter le LIVE et le SICK ou le DEAD et le DEAD4GOOD au besoin.

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