déclarations de retour lorsque vous faites Extrait Méthode

Question

Disons que vous avez une méthode très longue, comme ceci:

int monster() 
{ 
    int rc = 0; 

    // some statements ... 

    if (dragonSlayer.on_vacation()) { 
     cout << "We are screwed!\n"; 
     if (callTheKing() == true) 
      return 1; 
     else 
      return 2; 
    } else { 
     cout << "We are saved!\n"; 
     slayTheDragon(); 
    } 

    // rest of long method... 

    return rc; 
} 

et je travaille sur squelettisation le code. Je veux extraire la partie de dragon à terrassant

int handleDragon() { 
    if (dragonSlayer.on_vacation()) { 
     cout << "We are screwed!\n"; 
     if (callTheKing() == true) 
      return 1; 
     else 
      return 2; 
    } else { 
     cout << "We are saved!\n"; 
     slayTheDragon(); 
    } 

    return 0; // ? 
} 

et remplacer le code monstre() avec un appel à handleDragon().

Mais il y a un problème. Il y a une déclaration de retour au milieu de cette partie. Si je garde la partie où le code retour de handleDragon() est géré, il gardera la litière dans la grosse méthode. En plus des exceptions, existe-t-il un moyen élégant et sûr de refactoriser ce morceau de code par la méthode monstre? Comment ces types de situations devraient-ils être traités?

Answer 1

Retour 0 de la méthode handleDragon si le dragon slayer est disponible:

int handleDragon() { 
    if (dragonSlayer.on_vacation()) { 
     cout << "We are screwed!\n"; 
     if (callTheKing() == true) 
      return 1; 
     else 
      return 2; 
    } else { 
     cout << "We are saved!\n"; 
     slayTheDragon(); 
     return 0; 
    } 
} 

Puis retour dans la méthode monster, si la valeur de retour était supérieur à zéro, retourner cette valeur, porter autrement sur:

// some statements ... 

int handleDragonResult = handleDragon(); 
if (handleDragonResult > 0) { 
    return handleDragonResult; 
} 

// rest of long method... 

Vous devez également documenter la méthode handleDragon pour expliquer la valeur renvoyée.

Answer 2

Ne pourriez-vous faire ceci:

int handleDragon() { 
    int rc = 0; 

    if (dragonSlayer.on_vacation()) { 
     cout << "We are screwed!\n"; 
     if (callTheKing() == true) 
      rc = 1; 
     else 
      rc = 2; 
    } else { 
     cout << "We are saved!\n"; 
     slayTheDragon(); 
    } 

    return rc; 
} 

puis:

int monster() 
{ 
    int rc = 0; 

    // some statements ... 

    rc = handleDragon(); 

    // rest of long method... 

    return rc; 
} 

ou si vous voulez faire quelque chose avec le code de retour:

int monster() 
{ 
    int rc = 0; 

    // some statements ... 

    int handleDragonReturnCode = handleDragon(); 

    if(handleDragonReturnCode == 0) { 
     // do something 
    } 

    else { 
     // do something else 
    } 

    // rest of long method... 

    return rc; 
} 

Est-ce que tu veux? Sur une note générale, évitez d'utiliser nombres magiques comme 1 et 2 pour vos codes de retour. Utilisez les constantes, #define ou enum. Concernant return, essayez d'avoir un point de sortie de votre fonction. Comme vous l'avez découvert, avoir plusieurs instructions return peut rendre le refactoring difficile (ainsi que la compréhension de la logique, sauf si c'est vraiment simple).

Answer 3

enum DragonHandled { DHSuccess, DHKing, DHNoKing }; 

inline DragonHandled askForKing() 
{ 
    if (callTheKing()) 
     return DHKing; 
    else 
     return DHNoKing; 
} 

DragonHandled handleDragon() 
{ 
    if (dragonSlayer.on_vacation()) { 
     cout << "We are screwed!\n"; 
     return askForKing(); 
    } 
    cout << "We are saved!\n"; 
    slayTheDragon(); 
    return DHSuccess; 
} 

int monster() 
{ 
    some_statements(...); 

    DragonHandled handled = handleDragon(); 
    if(handled != DHSuccess) 
     return handled; // enum to int is an implicit cast 

    return more_statements(...); 
} 

• Sauf pour une fonction qui renvoie un nombre signé réelle, je ne retournerais pas int. Si le résultat a une signification, définissez ce sens correctement (c'est-à-dire: enum).
• Une fonction fait quelque chose, et quoi qu'il fasse, devrait être visible dans son nom.Il devrait y avoir un verbe au nom d'une fonction (handledragon(), callTheKing()). monsters n'est pas un verbe, ce n'est pas quelque chose que vous pouvez faire. Si je vois un identifiant monsters, je pense que c'est un conteneur pour les monstres.
• Vérification if(x == true) est tout simplement inutile, puisque if(x) est plus simple, plus simple et tout aussi vrai.

Answer 4

La question portait sur la stratégie, donc je pense que la réponse de Richard Fearn est bonne.

Pour en faire un modèle de refactoring il ressemblerait à quelque chose comme:

Contexte: Une section au milieu d'une méthode plus grande est à extraire.

Problème: La section contient des instructions de retour.

Solution:

1. Extrait du code à une nouvelle méthode de retour du même type que la méthode plus grande.
2. Trouvez une valeur de ce type qui ne signifie rien. Appelez cette valeur CONTINUE.
3. Ajoutez une instruction à la fin de la nouvelle méthode qui renvoie CONTINUE.
4. Dans la méthode plus grande, testez la valeur de retour de la nouvelle méthode pour CONTINUER. Si ce n'est pas le cas, retournez cette valeur.

Ceci serait l'approche principale. Comme prochaine étape, vous pouvez refactoriser les valeurs de retour de la nouvelle méthode à quelque chose de plus significatif (comme dans la réponse de sbi). Et vous devrez trouver un moyen de gérer le cas où le type de retour n'est pas un type scalaire ou simple, retournant un objet NULL ou un tel.