Par exemple, pour garder un processeur sous Android, je peux utiliser un code comme celui-ci:

PowerManager powerManager = (PowerManager)getSystemService(POWER_SERVICE);
WakeLock wakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "abc");
wakeLock.acquire();

mais je pense que les variables locales powerManageret wakeLockpeuvent être éliminés:

((PowerManager)getSystemService(POWER_SERVICE))
    .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "MyWakelockTag")
    .acquire();

Une scène similaire apparaît en mode alerte iOS, par exemple: de

UIAlertView *alert = [[UIAlertView alloc]
    initWithTitle:@"my title"
    message:@"my message"
    delegate:nil
    cancelButtonTitle:@"ok"
    otherButtonTitles:nil];
[alert show];

-(void)alertView:(UIAlertView *)alertView clickedButtonAtIndex:(NSInteger)buttonIndex{
    [alertView release];
}

à:

[[[UIAlertView alloc]
    initWithTitle:@"my title"
    message:@"my message"
    delegate:nil
    cancelButtonTitle:@"ok"
    otherButtonTitles:nil] show];

-(void)alertView:(UIAlertView *)alertView clickedButtonAtIndex:(NSInteger)buttonIndex{
    [alertView release];
}

Est-ce une bonne pratique d'éliminer une variable locale si elle est utilisée une seule fois dans le périmètre?

Le code est lu beaucoup plus souvent que ce qui est écrit, vous devriez donc avoir pitié de la pauvre âme qui devra le lire dans six mois (ce sera peut-être vous) et rechercher le code le plus clair et le plus facile à comprendre. À mon avis, la première forme, avec des variables locales, est beaucoup plus compréhensible. Je vois trois actions sur trois lignes, plutôt que trois actions sur une ligne.

Et si vous pensez optimiser quelque chose en vous débarrassant de variables locales, vous ne l'êtes pas. Un compilateur moderne mettra powerManagerdans un registre ¹ , qu'une variable locale soit utilisée ou non, pour appeler la newWakeLockméthode. La même chose est vraie pour wakeLock. Donc, vous vous retrouvez avec le même code compilé dans les deux cas.

¹ Si vous aviez beaucoup de code intermédiaire entre la déclaration et l'utilisation de la variable locale, cela pourrait aller sur la pile, mais c'est un détail mineur.

Certains commentaires très votants l'ont dit, mais aucune des réponses que j'ai vues ne l'a été. Je vais donc l'ajouter en tant que réponse. Votre facteur principal dans le choix de cette question est:

Débogage

Souvent, les développeurs consacrent beaucoup plus de temps et d’efforts au débogage qu’à l’écriture de code.

Avec les variables locales, vous pouvez:

• Point d'arrêt sur la ligne où il est affecté (avec toutes les autres décorations de points d'arrêt, telles que les points d'arrêt conditionnels, etc.)

• Inspecter / regarder / imprimer / changer la valeur de la variable locale

• Problèmes de capture dus aux lancers.

• Avoir des traces de pile lisibles (la ligne XYZ a une opération au lieu de 10)

Sans variables locales, toutes les tâches ci-dessus sont beaucoup plus difficiles, extrêmement difficiles ou carrément impossibles, selon votre débogueur.

En tant que tel, suivez la maxime infâme (écrivez le code de la même manière que si le développeur suivant le maintenait après vous-même était un fou fou qui sait où vous habitez) et commettez une erreur de débogage , ce qui signifie utiliser des variables locales. .

Seulement si cela rend le code plus facile à comprendre. Dans vos exemples, je pense que cela rend la lecture plus difficile.

L'élimination des variables dans le code compilé est une opération triviale pour tout compilateur respectable. Vous pouvez inspecter la sortie vous-même pour vérifier.

Votre question "est-ce une bonne pratique d'éliminer une variable locale si elle est utilisée une seule fois dans le périmètre?" teste les mauvais critères. L'utilité d'une variable locale ne dépend pas du nombre d'utilisations mais rend le code plus clair. L'étiquetage de valeurs intermédiaires avec des noms significatifs peut améliorer la clarté dans des situations telles que celle que vous présentez, car il divise le code en fragments plus petits et plus faciles à digérer.

À mon avis, le code non modifié que vous avez présenté est plus clair que votre code modifié et devrait donc rester inchangé. En fait, je considérerais d'extraire une variable locale de votre code modifié afin d'améliorer la clarté.

Je ne m'attendrais pas à un impact sur les performances des variables locales, et même s'il en existe une, elle sera probablement trop petite pour être considérée comme intéressante, à moins que le code ne soit dans une boucle très étroite dans une partie critique du programme.

Peut être.

Personnellement, j'hésiterais à éliminer les variables locales si une conversion de type est impliquée. Je trouve votre version compacte moins lisible car le nombre de crochets commence à s’approcher d’une limite mentale que j’ai. Il introduit même un nouvel ensemble de crochets qui n’était pas nécessaire dans la version légèrement plus détaillée qui utilise une variable locale.

La mise en évidence de la syntaxe fournie par l'éditeur de code peut atténuer ces problèmes dans une certaine mesure.

À mes yeux, c'est le meilleur compromis:

PowerManager powerManager = (PowerManager)getSystemService(POWER_SERVICE);
powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "abc").acquire();

Ainsi, garder une variable locale et abandonner l’autre. En C #, j'utiliserais le mot-clé var sur la première ligne car le transtypage en typographie fournit déjà les informations de type.

Bien que j'accepte la validité des réponses en faveur des variables locales, je vais me faire l'avocat du diable et présenter un point de vue opposé.

Personnellement, je suis un peu contre l’utilisation de variables locales à des fins purement documentaires, bien que cette raison indique elle-même que la pratique a de la valeur: les variables locales pseudo-documentent effectivement le code.

Dans votre cas, le problème principal est l’indentation et non l’absence de variables. Vous pouvez (et devriez) formater le code comme suit:

((PowerManager)getSystemService(POWER_SERVICE))
        .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,"MyWakelockTag")
        .acquire();

Comparez cela avec la version avec des variables locales:

PowerManager powerManager=(PowerManager)getSystemService(POWER_SERVICE);
WakeLock wakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,"abc");
wakeLock.acquire();

Avec les variables locales: les noms ajoutent des informations au lecteur et les types sont clairement spécifiés, mais les appels de méthode réels sont moins visibles et (à mon avis), la lecture est moins claire.

Sans utiliser de variables locales: c'est plus concis et les appels de méthode sont plus visibles, mais vous pouvez demander plus d'informations sur ce qui est renvoyé. Certains IDE peuvent toutefois vous le montrer.

Trois autres commentaires:

1. À mon avis, l'ajout de code qui n'a aucune utilisation fonctionnelle peut parfois être déroutant, car le lecteur doit se rendre compte qu'il n'a en réalité aucune utilisation fonctionnelle et qu'il est simplement là pour la "documentation". Par exemple, si cette méthode comptait 100 lignes, il ne serait pas évident que les variables locales (et donc le résultat de l'appel de la méthode) n'étaient pas nécessaires ultérieurement, à moins que vous ne lisiez la méthode complète.

2. L'ajout de variables locales signifie que vous devez spécifier leurs types, ce qui introduit dans le code une dépendance qui, autrement, ne serait pas présente. Si le type de retour des méthodes change de manière triviale (par exemple, il a été renommé), vous devrez mettre à jour votre code, alors que sans les variables locales, vous ne le feriez pas.

3. Le débogage pourrait être plus facile avec des variables locales si votre débogueur n'affiche pas les valeurs renvoyées par les méthodes. Mais la solution à cela est de corriger les défauts des débogueurs, pas de changer le code.

Il y a une tension de motivations ici. L'introduction de variables temporaires peut rendre le code plus facile à lire. Mais ils peuvent également empêcher, et rendre plus difficile à voir, d’autres refactorisations possibles, telles que Extract Method et Replace Temp by Query . Le cas échéant, ces derniers types de refactorisation offrent souvent des avantages bien supérieurs à ceux de la variable temp.

Fowler écrit à propos des motivations de ces derniers refactorings :

"Le problème avec les intérimaires est qu’ils sont temporaires et locaux. Parce qu’ils ne peuvent être vus que dans le contexte de la méthode dans laquelle ils sont utilisés, ils ont tendance à encourager les méthodes plus longues, car c’est le seul moyen d’atteindre l’intérimaire. Par En remplaçant le temp par une méthode de requête, toute méthode de la classe peut accéder à l'information. Cela aide beaucoup à créer un code plus propre pour la classe. "

Donc, oui, utilisez les temps temporaires s'ils rendent le code plus lisible, surtout s'il s'agit d'une norme locale pour vous et votre équipe. Mais sachez que cela peut parfois rendre plus difficile la découverte d’améliorations alternatives plus importantes. Si vous pouvez développer votre capacité à sentir quand il est utile de vous passer de tels moments et si vous vous sentez plus à l'aise, alors c'est probablement une bonne chose.

FWIW J'ai personnellement évité de lire le livre de Fowler "Refactoring" pendant dix ans parce que je pensais qu'il n'y avait pas grand-chose à dire sur des sujets aussi simples. J'avais complètement tort. Lorsque j'ai fini par le lire, cela a changé mes pratiques de codage quotidiennes, pour le meilleur.

Bien, c’est une bonne idée d’éliminer les variables locales si vous pouvez ainsi rendre le code plus lisible. Votre longue ligne n’est pas lisible, mais elle suit un joli style OO. Si vous pouviez le réduire à quelque chose comme

acquireWakeLock(POWER_SERVICE, PARTIAL, "abc");

alors je dirais que ce serait probablement une bonne idée. Peut-être pouvez-vous introduire des méthodes d'assistance pour résumer le problème en quelque chose de similaire; si un code comme celui-ci apparaît plusieurs fois, il pourrait en valoir la peine.

Considérons la loi de Demeter ici. L'article de Wikipédia sur LoD indique ce qui suit:

La loi de Demeter pour les fonctions exige qu'une méthode m d'un objet O ne puisse invoquer que les méthodes des types d'objets suivants: [2]

1. O lui-même
2. paramètres de m
3. Tous les objets créés / instanciés dans m
4. Les objets composants directs de O
5. Une variable globale, accessible par O, dans le domaine de m

L’application de cette loi a notamment pour conséquence d’éviter d’invoquer des méthodes d’objets renvoyés par d’autres méthodes dans une longue chaîne pointillée, comme le montre le deuxième exemple ci-dessus:

((PowerManager)getSystemService(POWER_SERVICE)).newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,"MyWakelockTag").acquire();

C'est vraiment difficile de comprendre ce que cela fait. Pour le comprendre, vous devez comprendre ce que fait chaque procédure, puis déchiffrer quelle fonction est appelée sur quel objet. Le premier bloc de code

PowerManager powerManager=(PowerManager)getSystemService(POWER_SERVICE);
WakeLock wakeLock=powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,"abc");
wakeLock.acquire();

indique clairement ce que vous essayez de faire: vous obtenez un objet PowerManager, vous obtenez un objet WakeLock à partir de PowerManager, puis vous acquérez le wakeLock. La règle n ° 3 de LoD ci-dessus est la suivante: vous instanciez ces objets dans votre code et vous pouvez donc les utiliser pour obtenir les résultats dont vous avez besoin.

Une autre façon de penser est peut-être de se rappeler que lors de la création d'un logiciel, vous devez écrire par souci de clarté et non par souci de concision. 90% de tout le développement de logiciels est la maintenance. N'écrivez jamais un morceau de code que vous ne voudriez pas maintenir.

Bonne chance.

Une chose à noter est que le code est lu fréquemment, à différentes "profondeurs". Ce code:

PowerManager powerManager = (PowerManager)getSystemService(POWER_SERVICE);
WakeLock wakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "abc");
wakeLock.acquire();

est facile à "écrémer". C'est 3 déclarations. Nous commençons par un PowerManager. Ensuite, nous arrivons avec un WakeLock. Ensuite nous acquirele wakeLock. Je peux voir cela très facilement simplement en regardant le début de chaque ligne; Les affectations de variables simples sont vraiment faciles à reconnaître partiellement comme "Type varName = ..." et il suffit de parcourir mentalement le "...". De même, la dernière affirmation n’est évidemment pas la forme de la cession, mais elle n’implique que deux noms, l’essentiel est donc immédiatement apparent. Souvent, c'est tout ce dont j'avais besoin de savoir si j'essayais simplement de répondre "à quoi sert ce code?" à un niveau élevé.

Si je suis à la recherche d'un bogue subtil dont je pense qu'il est ici, il est évident que je devrai l'examiner plus en détail et que je me souviendrai en réalité des "..." Mais la structure de déclaration séparée m'aide toujours à faire cette déclaration à la fois (particulièrement utile si je dois approfondir la mise en œuvre des éléments appelés dans chaque déclaration; lorsque je reviens, j'ai bien compris «une unité» et peut ensuite passer à la déclaration suivante).

((PowerManager)getSystemService(POWER_SERVICE))
    .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "MyWakelockTag")
    .acquire();

Maintenant, tout est une déclaration. La structure de niveau supérieur n'est pas si facile à lire en écrivant; dans la version originale du PO, sans sauts de ligne ni indentation pour communiquer visuellement une structure, il aurait fallu compter les parenthèses pour le décoder en une séquence de 3 étapes. Si certaines expressions à plusieurs parties sont imbriquées les unes dans les autres plutôt que disposées en chaîne d'appels de méthode, elles risquent de ressembler à cela. Je dois donc faire attention à ne pas faire confiance à cela sans compter les parenthèses. Et si je me fie à l’indentation et que je passe à la dernière chose présumée de tout cela, que .acquire()me dit-il en soi?

Parfois cependant, cela peut être ce que vous voulez. Si j'applique votre transformation à mi-chemin et écrit:

WakeLock wakeLock =
     ((PowerManeger)getSystemService(POWER_SERVICE))
    .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "MyWakeLockTage");
wakeLock.acquire();

Maintenant, cela communique en un rapide "obtenir un WakeLock, puis acquireil". Encore plus simple que la première version. Il est immédiatement évident que la chose acquise est un WakeLock. Si obtenir un PowerManagerest juste un détail assez insignifiant par rapport à l’objet de ce code, mais wakeLockc’est important, alors il pourrait être utile d’enterrer le PowerManagercontenu, il est donc naturel de le survoler si vous essayez juste d’obtenir rapidement idée de ce que ce code fait. Et ne pas le nommer communique qu'il n'est utilisé qu'une seule fois, et parfois c'est ce qui est important (si le reste de la portée est très longue, je devrais lire tout cela pour savoir si elle est utilisée à nouveau; bien que l'utilisation de sous-portées explicites puisse être un autre moyen de résoudre ce problème, si votre langage le permet

Ce qui montre que tout dépend du contexte et de ce que vous voulez communiquer. Comme pour l'écriture de prose en langage naturel, il existe toujours de nombreuses façons d'écrire un code donné qui sont fondamentalement équivalentes en termes de contenu d'information. Comme pour l'écriture de prose en langage naturel, vous ne devez généralement pas appliquer de règles mécanistes telles que "éliminer toute variable locale ne se produisant qu'une seule fois". Plutôt commentvous choisissez d'écrire votre code mettra l'accent sur certaines choses et de moins souligner d'autres. Vous devez vous efforcer de faire ces choix consciemment (y compris le choix d'écrire du code moins lisible pour des raisons techniques parfois), en fonction de ce que vous voulez réellement souligner. Pensez en particulier à ce qui sera utile aux lecteurs qui doivent juste "comprendre l'essentiel" de votre code (à différents niveaux), car cela se produira beaucoup plus souvent qu'une lecture très proche expression par expression.

C'est même un antipattern avec son propre nom: Train Wreck . Plusieurs raisons d'éviter le remplacement ont déjà été indiquées:

• Plus difficile à lire
• Plus difficile à déboguer (pour surveiller les variables et détecter les emplacements des exceptions)
• Violation de la loi de Demeter (LoD)

Considérez si cette méthode en sait trop sur les autres objets. Le chaînage de méthodes est une alternative qui pourrait vous aider à réduire le couplage.

Rappelez-vous également que les références aux objets sont vraiment bon marché.

La question posée est "Devrions-nous éliminer les variables locales si nous le pouvons"?

Non, vous ne devriez pas éliminer simplement parce que vous le pouvez.

Vous devez suivre les directives de codage dans votre boutique.

La plupart du temps, les variables locales facilitent la lecture et le débogage du code.

J'aime ce que tu as fait avec PowerManager powerManager. Pour moi, un minuscule de la classe signifie qu'il ne s'agit que d'un usage unique.

Lorsque vous ne devriez pas le faire, la variable doit conserver des ressources coûteuses. De nombreuses langues ont une syntaxe pour une variable locale qui doit être nettoyée / publiée. En C #, il utilise.

using(SQLconnection conn = new SQLconnnection())
{
    using(SQLcommand cmd = SQLconnnection.CreateCommand())
    {
    }
}

Un aspect important n'a pas été mentionné dans les autres réponses: chaque fois que vous ajoutez une variable, vous introduisez un état mutable. C'est généralement une mauvaise chose car cela rend votre code plus complexe et donc plus difficile à comprendre et à tester. Bien entendu, plus la portée de la variable est petite, plus le problème est petit.

Ce que vous voulez n’est en réalité pas une variable dont la valeur peut être modifiée, mais une constante temporaire. Par conséquent, songez à exprimer cela dans votre code si votre langue le permet. En Java, vous pouvez utiliser finalet en C ++, vous pouvez utiliser const. Dans la plupart des langages fonctionnels, il s'agit du comportement par défaut.

Il est vrai que les variables locales sont le type d’état modifiable le moins préjudiciable. Les variables membres peuvent causer beaucoup plus de problèmes et les variables statiques sont encore pires. Néanmoins, je trouve important d'exprimer aussi précisément que possible dans votre code ce qu'il est censé faire. Et il existe une énorme différence entre une variable qui pourrait être modifiée ultérieurement et un simple nom pour un résultat intermédiaire. Donc, si votre langage vous permet d'exprimer cette différence, faites-le.