language-agnostic - within - one return per method java




Une fonction devrait-elle avoir une seule déclaration de retour? (20)

Y a-t-il de bonnes raisons pour lesquelles il est préférable d'avoir une seule déclaration de retour dans une fonction?

Ou est-ce correct de revenir d'une fonction dès qu'il est logique de le faire, ce qui signifie qu'il peut y avoir plusieurs instructions de retour dans la fonction?


Y a-t-il de bonnes raisons pour lesquelles il est préférable d'avoir une seule déclaration de retour dans une fonction?

Oui , il y a:

  • Le point de sortie unique donne un excellent endroit pour affirmer vos post-conditions.
  • Pouvoir mettre un point d'arrêt du débogueur sur le retour à la fin de la fonction est souvent utile.
  • Moins de retours signifie moins de complexité. Le code linéaire est généralement plus simple à comprendre.
  • Si essayer de simplifier une fonction à un seul retour entraîne une complexité, il est alors intéressant de refactoriser des fonctions plus petites, plus générales et plus faciles à comprendre.
  • Si vous êtes dans une langue sans destructeur ou si vous n'utilisez pas RAII, alors un seul retour réduit le nombre de places à nettoyer.
  • Certaines langues nécessitent un seul point de sortie (par exemple, Pascal et Eiffel).

La question est souvent posée comme une fausse dichotomie entre des déclarations multiples ou des déclarations si imbriquées. Il y a presque toujours une troisième solution très linéaire (pas d'imbrication profonde) avec un seul point de sortie.

Mise à jour : Apparemment, les directives MISRA favorisent la sortie unique .

Pour être clair, je ne dis pas que c'est toujours faux d'avoir plusieurs retours. Mais étant donné des solutions par ailleurs équivalentes, il y a beaucoup de bonnes raisons de préférer celle avec un seul retour.


Avoir un seul point de sortie offre un avantage dans le débogage, car il vous permet de définir un seul point d'arrêt à la fin d'une fonction pour voir quelle valeur va réellement être retournée.


Comme le note Kent Beck en discutant des clauses de garde dans les modèles de mise en œuvre, une routine a un seul point d'entrée et de sortie ...

"était d'empêcher la confusion possible en sautant et en sortant de nombreux endroits dans la même routine.Il était logique de l'appliquer aux programmes FORTRAN ou en assembleur écrits avec beaucoup de données globales où même comprendre quelles instructions étaient exécutées était un travail difficile. Avec de petites méthodes et surtout des données locales, il est inutilement conservateur. "

Je trouve une fonction écrite avec des clauses de garde beaucoup plus facile à suivre qu'une longue série imbriquée d'instructions if then else .


Dans une fonction qui n'a pas d'effets secondaires, il n'y a pas de bonne raison d'avoir plus d'un seul retour et vous devriez les écrire dans un style fonctionnel. Dans une méthode avec des effets secondaires, les choses sont plus séquentielles (indexées dans le temps), donc vous écrivez dans un style impératif, en utilisant l'instruction return comme une commande pour arrêter l'exécution.

En d'autres termes, quand c'est possible, privilégiez ce style

return a > 0 ?
  positively(a):
  negatively(a);

sur ceci

if (a > 0)
  return positively(a);
else
  return negatively(a);

Si vous vous trouvez en train d'écrire plusieurs couches de conditions imbriquées, il y a probablement un moyen de refactoriser cela, en utilisant une liste de prédicats par exemple. Si vous trouvez que vos ifs et elses sont très éloignés syntaxiquement, vous voudrez peut-être décomposer cela en plus petites fonctions. Un bloc conditionnel qui couvre plus d'un écran de texte est difficile à lire.

Il n'y a pas de règle absolue qui s'applique à toutes les langues. Quelque chose comme avoir une seule déclaration de retour ne rendra pas votre code bon. Mais bon code aura tendance à vous permettre d'écrire vos fonctions de cette façon.


J'ai souvent plusieurs déclarations au début d'une méthode pour retourner dans des situations "faciles". Par exemple, ceci:

public void DoStuff(Foo foo)
{
    if (foo != null)
    {
        ...
    }
}

... peut être rendu plus lisible (IMHO) comme ceci:

public void DoStuff(Foo foo)
{
    if (foo == null) return;

    ...
}

Alors oui, je pense que c'est bien d'avoir plusieurs "points de sortie" d'une fonction / méthode.


Je dirais qu'il serait incroyablement imprudent de décider arbitrairement contre plusieurs points de sortie car j'ai trouvé que la technique était utile dans la pratique encore et encore , en fait j'ai souvent refaçonné le code existant à plusieurs points de sortie pour plus de clarté. Nous pouvons comparer les deux approches ainsi:

string fooBar(string s, int? i) {
  string ret = "";
  if(!string.IsNullOrEmpty(s) && i != null) {
    var res = someFunction(s, i);

    bool passed = true;
    foreach(var r in res) {
      if(!r.Passed) {
        passed = false;
        break;
      }
    }

    if(passed) {
      // Rest of code...
    }
  }

  return ret;
}

Comparez cela au code où plusieurs points de sortie sont autorisés: -

string fooBar(string s, int? i) {
  var ret = "";
  if(string.IsNullOrEmpty(s) || i == null) return null;

  var res = someFunction(s, i);

  foreach(var r in res) {
      if(!r.Passed) return null;
  }

  // Rest of code...

  return ret;
}

Je pense que ce dernier est considérablement plus clair. Autant que je sache, la critique de plusieurs points de sortie est un point de vue plutôt archaïque ces temps-ci.


Je l'ai vu dans les normes de codage pour C ++ qui étaient un hang-over de C, comme si vous n'aviez pas RAII ou toute autre gestion automatique de la mémoire, alors vous devez nettoyer pour chaque retour, ce qui signifie couper et coller du nettoyage ou d'un goto (logiquement le même que 'finally' dans les langages gérés), les deux étant considérés comme mauvais. Si vos pratiques consistent à utiliser des pointeurs et des collections intelligents en C ++ ou dans un autre système de mémoire automatique, il n'y a pas de raison majeure pour cela, et il s'agit uniquement de la lisibilité, et plus d'un jugement.


Je me force à n'utiliser qu'une seule déclaration de return , car cela va dans un sens générer une odeur de code. Laisse-moi expliquer:

function isCorrect($param1, $param2, $param3) {
    $toret = false;
    if ($param1 != $param2) {
        if ($param1 == ($param3 * 2)) {
            if ($param2 == ($param3 / 3)) {
                $toret = true;
            } else {
                $error = 'Error 3';
            }
        } else {
            $error = 'Error 2';
        }
    } else {
        $error = 'Error 1';
    }
    return $toret;
}

(Les conditions sont arbritaires ...)

Plus il y a de conditions, plus la fonction est grande, plus il est difficile à lire. Donc, si vous êtes sensible à l'odeur du code, vous le réaliserez, et vous voulez refactoriser le code. Deux solutions possibles sont:

  • Retours multiples
  • Refactorisation en fonctions séparées

Retours multiples

function isCorrect($param1, $param2, $param3) {
    if ($param1 == $param2)       { $error = 'Error 1'; return false; }
    if ($param1 != ($param3 * 2)) { $error = 'Error 2'; return false; }
    if ($param2 != ($param3 / 3)) { $error = 'Error 3'; return false; }
    return true;
}

Séparer les fonctions

function isEqual($param1, $param2) {
    return $param1 == $param2;
}

function isDouble($param1, $param2) {
    return $param1 == ($param2 * 2);
}

function isThird($param1, $param2) {
    return $param1 == ($param2 / 3);
}

function isCorrect($param1, $param2, $param3) {
    return !isEqual($param1, $param2)
        && isDouble($param1, $param3)
        && isThird($param2, $param3);
}

Certes, il est plus long et un peu brouillon, mais dans le processus de refactorisation de la fonction de cette façon, nous avons

  • créé un certain nombre de fonctions réutilisables,
  • rendu la fonction plus lisible par l'homme, et
  • l'accent des fonctions est sur pourquoi les valeurs sont correctes.

Je travaille actuellement sur une base de code où deux des personnes qui travaillent dessus s'abonnent aveuglément à la théorie du «point unique de sortie» et je peux vous dire que par expérience, c'est une horrible pratique horrible. Cela rend le code extrêmement difficile à maintenir et je vais vous montrer pourquoi.

Avec la théorie du «point de sortie unique», vous obtenez inévitablement un code qui ressemble à ceci:

function()
{
    HRESULT error = S_OK;

    if(SUCCEEDED(Operation1()))
    {
        if(SUCCEEDED(Operation2()))
        {
            if(SUCCEEDED(Operation3()))
            {
                if(SUCCEEDED(Operation4()))
                {
                }
                else
                {
                    error = OPERATION4FAILED;
                }
            }
            else
            {
                error = OPERATION3FAILED;
            }
        }
        else
        {
            error = OPERATION2FAILED;
        }
    }
    else
    {
        error = OPERATION1FAILED;
    }

    return error;
}

Non seulement cela rend le code très difficile à suivre, mais maintenant, disons plus tard, vous devez revenir en arrière et ajouter une opération entre 1 et 2. Vous devez mettre en retrait à peu près toute la fonction de panique, et bonne chance en vous assurant vos conditions if / else et vos accolades sont correctement mises en correspondance.

Cette méthode rend la maintenance du code extrêmement difficile et sujette aux erreurs.


Ma préférence serait pour une seule sortie à moins que cela ne complique vraiment les choses. J'ai trouvé que dans certains cas, plusieurs points d'existence peuvent masquer d'autres problèmes de conception plus importants:

public void DoStuff(Foo foo)
{
    if (foo == null) return;
}

En voyant ce code, je demanderais immédiatement:

  • Est-ce que 'foo' est toujours nul?
  • Si oui, combien de clients de 'DoStuff' appellent la fonction avec un 'foo' nul?

Selon les réponses à ces questions, il se pourrait que

  1. la vérification est inutile car elle n'est jamais vraie (c'est-à-dire qu'elle devrait être une assertion)
  2. la vérification est très rarement vraie et il peut donc être préférable de changer ces fonctions d'appel spécifiques car elles devraient probablement prendre d'autres mesures de toute façon.

Dans les deux cas ci-dessus, le code peut probablement être retravaillé avec une assertion pour s'assurer que 'foo' n'est jamais nul et que les appelants concernés ont changé.

Il y a deux autres raisons (spécifiques au code C ++, je pense) où les multiples existent peuvent avoir un effet négatif . Ce sont la taille du code et les optimisations du compilateur.

Un objet C ++ non-POD dans la portée à la sortie d'une fonction aura son destructeur appelé. Lorsqu'il y a plusieurs déclarations de retour, il peut y avoir des objets différents dans la portée et donc la liste des destructeurs à appeler sera différente. Le compilateur a donc besoin de générer du code pour chaque déclaration de retour:

void foo (int i, int j) {
  A a;
  if (i > 0) {
     B b;
     return ;   // Call dtor for 'b' followed by 'a'
  }
  if (i == j) {
     C c;
     B b;
     return ;   // Call dtor for 'b', 'c' and then 'a'
  }
  return 'a'    // Call dtor for 'a'
}

Si la taille du code est un problème - alors cela peut être quelque chose qui vaut la peine d'être évité.

L'autre problème se rapporte à "Named Return Value OptimiZation" (alias Copy Elision, ISO C ++ '03 12.8 / 15). C ++ permet à une implémentation d'ignorer l'appel du constructeur de copie s'il peut:

A foo () {
  A a1;
  // do something
  return a1;
}

void bar () {
  A a2 ( foo() );
}

En prenant simplement le code tel quel, l'objet 'a1' est construit dans 'foo' et ensuite sa construction de copie sera appelée pour construire 'a2'. Cependant, l'élision de la copie permet au compilateur de construire 'a1' au même endroit de la pile que 'a2'. Il n'y a donc pas besoin de "copier" l'objet lorsque la fonction revient.

Plusieurs points de sortie compliquent le travail du compilateur en essayant de détecter cela, et au moins pour une version relativement récente de VC ++, l'optimisation n'a pas eu lieu là où le corps de la fonction avait plusieurs retours. Voir Optimisation de la valeur de retour nommée dans Visual C ++ 2005 pour plus de détails.


Personne n'a mentionné ou cité Code Complete donc je vais le faire.

17,1 retour

Minimisez le nombre de retours dans chaque routine . Il est plus difficile de comprendre une routine si, en la lisant en bas, vous ignorez la possibilité qu'elle soit revenue quelque part au-dessus.

Utilisez un retour quand il améliore la lisibilité . Dans certaines routines, une fois que vous connaissez la réponse, vous voulez la renvoyer immédiatement à la routine d'appel. Si la routine est définie de telle sorte qu'elle ne nécessite aucun nettoyage, ne pas retourner immédiatement signifie que vous devez écrire plus de code.


La programmation structurée indique que vous ne devriez avoir qu'une déclaration de retour par fonction. C'est pour limiter la complexité. Beaucoup de gens comme Martin Fowler soutiennent qu'il est plus simple d'écrire des fonctions avec plusieurs déclarations de retour. Il présente cet argument dans le livre de refactoring classique qu'il a écrit. Cela fonctionne bien si vous suivez ses autres conseils et écrivez de petites fonctions. Je suis d'accord avec ce point de vue et seuls les puristes de la programmation structurée stricte adhèrent à des déclarations de rendement unique par fonction.


I lean towards using guard clauses to return early and otherwise exit at the end of a method. The single entry and exit rule has historical significance and was particularly helpful when dealing with legacy code that ran to 10 A4 pages for a single C++ method with multiple returns (and many defects). More recently, accepted good practice is to keep methods small which makes multiple exits less of an impedance to understanding. In the following Kronoz example copied from above, the question is what occurs in //Rest of code... ?:

void string fooBar(string s, int? i) {

  if(string.IsNullOrEmpty(s) || i == null) return null;

  var res = someFunction(s, i);

  foreach(var r in res) {
      if(!r.Passed) return null;
  }

  // Rest of code...

  return ret;
}

I realise the example is somewhat contrived but I would be tempted to refactor the foreach loop into a LINQ statement that could then be considered a guard clause. Again, in a contrived example the intent of the code isn't apparent and someFunction() may have some other side effect or the result may be used in the // Rest of code... .

if (string.IsNullOrEmpty(s) || i == null) return null;
if (someFunction(s, i).Any(r => !r.Passed)) return null;

Giving the following refactored function:

void string fooBar(string s, int? i) {

  if (string.IsNullOrEmpty(s) || i == null) return null;
  if (someFunction(s, i).Any(r => !r.Passed)) return null;

  // Rest of code...

  return ret;
}

I vote for Single return at the end as a guideline. This helps a common code clean-up handling ... For example, take a look at the following code ...

void ProcessMyFile (char *szFileName)
{
   FILE *fp = NULL;
   char *pbyBuffer = NULL:

   do {

      fp = fopen (szFileName, "r");

      if (NULL == fp) {

         break;
      }

      pbyBuffer = malloc (__SOME__SIZE___);

      if (NULL == pbyBuffer) {

         break;
      }

      /*** Do some processing with file ***/

   } while (0);

   if (pbyBuffer) {

      free (pbyBuffer);
   }

   if (fp) {

      fclose (fp);
   }
}

If you end up with more than a few returns there may be something wrong with your code. Otherwise I would agree that sometimes it is nice to be able to return from multiple places in a subroutine, especially when it make the code cleaner.

Perl 6: Bad Example

sub Int_to_String( Int i ){
  given( i ){
    when 0 { return "zero" }
    when 1 { return "one" }
    when 2 { return "two" }
    when 3 { return "three" }
    when 4 { return "four" }
    ...
    default { return undef }
  }
}

would be better written like this

Perl 6: Good Example

@Int_to_String = qw{
  zero
  one
  two
  three
  four
  ...
}
sub Int_to_String( Int i ){
  return undef if i < 0;
  return undef unless i < @Int_to_String.length;
  return @Int_to_String[i]
}

Note this is was just a quick example


Multiple exit points are fine for small enough functions -- that is, a function that can be viewed on one screen length on its entirety. If a lengthy function likewise includes multiple exit points, it's a sign that the function can be chopped up further.

That said I avoid multiple-exit functions unless absolutely necessary . I have felt pain of bugs that are due to some stray return in some obscure line in more complex functions.


One good reason I can think of is for code maintenance: you have a single point of exit. If you want to change the format of the result,..., it's just much simpler to implement. Also, for debugging, you can just stick a breakpoint there :)

Having said that, I once had to work in a library where the coding standards imposed 'one return statement per function', and I found it pretty tough. I write lots of numerical computations code, and there often are 'special cases', so the code ended up being quite hard to follow...


Single exit point - all other things equal - makes code significantly more readable. But there's a catch: popular construction

resulttype res;
if if if...
return res;

is a fake, "res=" is not much better than "return". It has single return statement, but multiple points where function actually ends.

If you have function with multiple returns (or "res="s), it's often a good idea to break it into several smaller functions with single exit point.


This is probably an unusual perspective, but I think that anyone who believes that multiple return statements are to be favoured has never had to use a debugger on a microprocessor that supports only 4 hardware breakpoints. ;-)

While the issues of "arrow code" are completely correct, one issue that seems to go away when using multiple return statements is in the situation where you are using a debugger. You have no convenient catch-all position to put a breakpoint to guarantee that you're going to see the exit and hence the return condition.


You know the adage - beauty is in the eyes of the beholder .

Some people swear by NetBeans and some by IntelliJ IDEA , some by Python and some by PHP .

In some shops you could lose your job if you insist on doing this:

public void hello()
{
   if (....)
   {
      ....
   }
}

The question is all about visibility and maintainability.

I am addicted to using boolean algebra to reduce and simplify logic and use of state machines. However, there were past colleagues who believed my employ of "mathematical techniques" in coding is unsuitable, because it would not be visible and maintainable. And that would be a bad practice. Sorry people, the techniques I employ is very visible and maintainable to me - because when I return to the code six months later, I would understand the code clearly rather seeing a mess of proverbial spaghetti.

Hey buddy (like a former client used to say) do what you want as long as you know how to fix it when I need you to fix it.

I remember 20 years ago, a colleague of mine was fired for employing what today would be called agile development strategy. He had a meticulous incremental plan. But his manager was yelling at him "You can't incrementally release features to users! You must stick with the waterfall ." His response to the manager was that incremental development would be more precise to customer's needs. He believed in developing for the customers needs, but the manager believed in coding to "customer's requirement".

We are frequently guilty for breaking data normalization, MVP and MVC boundaries. We inline instead of constructing a function. We take shortcuts.

Personally, I believe that PHP is bad practice, but what do I know. All the theoretical arguments boils down to trying fulfill one set of rules

quality = precision, maintainability and profitability.

All other rules fade into the background. And of course this rule never fades:

Laziness is the virtue of a good programmer.





coding-style