Il codice viene letto molto più spesso di quanto non sia scritto, quindi dovresti avere pietà della povera anima che dovrà leggere il codice tra sei mesi (potrebbe essere tu) e cercare il codice più chiaro e facile da capire . Secondo me, la prima forma, con variabili locali, è molto più comprensibile. Vedo tre azioni su tre righe, piuttosto che tre azioni su una riga.

E se pensi di ottimizzare qualcosa eliminando le variabili locali, non lo sei. Un moderno compilatore metterà powerManager in un registro ¹ , se una variabile locale è usata o meno, per chiamare il metodo newWakeLock . Lo stesso vale per wakeLock . Quindi in entrambi i casi si finisce con lo stesso codice compilato.

¹ Se hai un sacco di codice intercorrente tra la dichiarazione e l'uso della variabile locale, potrebbe andare in pila, ma è un dettaglio minore.

Alcuni commenti di alto livello hanno affermato questo, ma nessuna delle risposte che ho visto ha fatto, quindi la aggiungerò come risposta. Il tuo principale fattore nel decidere questo problema è:

Debuggability

Spesso, gli sviluppatori dedicano molto più tempo e impegno al debugging rispetto alla scrittura del codice.

Con le variabili locali , puoi:

• Punto di interruzione sulla linea in cui è stato assegnato (con tutte le altre decorazioni del punto di interruzione, come il punto di interruzione condizionale, ecc ...)

• Ispeziona / guarda / stampa / cambia il valore della variabile locale

• Problemi di catch che si verificano a causa di cast.

• ha tracce di stack leggibili (la riga XYZ ha una operazione invece di 10)

Senza variabili locali, tutte le attività sopra elencate sono molto più difficili, estremamente difficili o addirittura impossibili, a seconda del tuo debugger.

Come tale, segui la famigerata massima (scrivi il codice in un modo che vorresti se il prossimo sviluppatore per mantenerla dopo te stesso fosse un pazzo pazzo che sa dove vivi), e sbagli sul lato di più facile debuggability , che significa utilizza le variabili locali .

Solo se rende il codice più facile da capire. Nei tuoi esempi, penso che sia più difficile da leggere.

Eliminare le variabili nel codice compilato è un'operazione banale per qualsiasi compilatore rispettabile. Puoi verificare tu stesso l'output per verificare.

La tua domanda "è una buona pratica eliminare una variabile locale se è usata una sola volta nell'ambito?" sta testando i criteri sbagliati. L'utilità di una variabile locale non dipende dal numero di volte in cui viene utilizzata, ma dal fatto se rende il codice più chiaro. Etichettare valori intermedi con nomi significativi può migliorare la chiarezza in situazioni come quella che si presenta poiché scompone il codice in pezzi più piccoli, più digeribili.

A mio avviso, il codice non modificato che hai presentato è più chiaro del tuo codice modificato e quindi dovrebbe essere lasciato invariato. In effetti, prenderei in considerazione l'estrazione di una variabile locale dal codice modificato per migliorare la chiarezza.

Non mi aspetto alcun impatto sulle prestazioni delle variabili locali, e anche se ce n'è una, è probabile che sia troppo piccola per essere presa in considerazione a meno che il codice non sia in un ciclo molto stretto in una parte critica del programma.

Forse.

Personalmente esiterei ad eliminare le variabili locali se è coinvolto un typecast. Trovo la tua versione compatta meno leggibile in quanto il numero di parentesi inizia ad avvicinarsi a un limite mentale che ho. Introduce anche un nuovo set di parentesi che non era necessario nella versione leggermente più dettagliata usando una variabile locale.

L'evidenziazione della sintassi fornita dall'editor di codice potrebbe in qualche misura attenuare tali preoccupazioni.

Ai miei occhi, questo è il compromesso migliore:

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

Così mantenendo una variabile locale e abbandonando l'altra. In C # userei la parola chiave var sulla prima riga poiché il typecast fornisce già le informazioni sul tipo.

Anche se accetto la validità delle risposte a favore delle variabili locali, interpreterò l'avvocato del diavolo e presenterò una visione controcorrente.

Personalmente sono contrario all'utilizzo di variabili locali puramente per scopi di documentazione, sebbene questa stessa ragione indichi che la pratica ha un valore: le variabili locali effettivamente pseudo-documentano il codice.

Nel tuo caso, il problema principale è l'indentazione e non l'assenza di variabili. Potresti (e dovresti) formattare il codice come segue:

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

Confrontalo con la versione con le variabili locali:

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

Con le variabili locali: i nomi aggiungono alcune informazioni al lettore e i tipi sono chiaramente specificati, ma le chiamate al metodo effettivo sono meno visibili e (a mio parere) non vengono letti in modo chiaro.

Senza utilizzare le variabili locali: è più conciso e le chiamate al metodo sono più visibili, ma potresti chiedere maggiori informazioni su ciò che viene restituito. Alcuni IDE possono mostrarti questo comunque.

Tre ulteriori commenti:

1. A mio parere, l'aggiunta di codice che non ha un uso funzionale può a volte essere fonte di confusione poiché il lettore deve rendersi conto che non ha davvero alcun uso funzionale ed è semplicemente lì per "documentazione". Ad esempio, se questo metodo fosse lungo 100 righe, non sarebbe ovvio che le variabili locali (e quindi il risultato della chiamata al metodo) non fossero necessarie in un secondo momento a meno che non si leggesse l'intero metodo.

2. Aggiungere le variabili locali e significa che devi specificare i loro tipi e questo introduce una dipendenza nel codice che altrimenti non sarebbe presente. Se il tipo di ritorno dei metodi è cambiato banalmente (ad es. È stato rinominato) dovresti aggiornare il tuo codice, mentre senza le variabili locali non lo faresti.

3. Il debugging sarà più semplice con le variabili locali. Ma la soluzione è quella di correggere le carenze nei debugger, non modificare il codice.

Qui c'è una tensione di motivazione. L'introduzione di variabili temporanee può rendere il codice più facile da leggere. Ma possono anche prevenire e rendere più difficile vedere altri possibili refactoring, come Metodo di estrazione e Sostituisci temp con query . Questi ultimi tipi di refactoring, se del caso, spesso offrono benefici molto maggiori rispetto alla variabile temp.

Sulle motivazioni di questi ultimi refactoring, Fowler scrive :

"The problem with temps is that they are temporary and local. Because they can be seen only in the context of the method in which they are used, temps tend to encourage longer methods, because that’s the only way you can reach the temp. By replacing the temp with a query method, any method in the class can get at the information. That helps a lot in coming up with cleaner code for the class."

Quindi, sì, usa le temp se rendono il codice più leggibile, specialmente se questa è una norma locale per te e il tuo team. Ma sappi che farlo a volte può rendere più difficile individuare miglioramenti alternativi più grandi. Se riesci a sviluppare la tua capacità di percepire quando vale la pena andare senza tali temp e diventare più a tuo agio nel farlo, allora è probabilmente una buona cosa.

FWIW Personalmente ho evitato di leggere il libro di Fowler "Refactoring" per dieci anni perché immaginavo che non ci fosse molto da dire su argomenti relativamente semplici. Ho sbagliato completamente. Quando alla fine l'ho letto, ha cambiato le mie pratiche quotidiane di codice, tanto per il meglio.

Bene, è una buona idea eliminare le variabili locali se è possibile rendere il codice più leggibile in tal modo. Quella lunga copertina non è leggibile, ma segue uno stile OO piuttosto prolisso. Se potessi ridurlo a qualcosa di simile

acquireWakeLock(POWER_SERVICE, PARTIAL, "abc");

quindi direi che probabilmente sarebbe una buona idea. Forse puoi introdurre metodi di supporto per ridurli a qualcosa di simile; se il codice come questo appare più volte allora potrebbe valere la pena.

Consideriamo la Legge di Demetra qui. Nell'articolo di Wikipedia su LoD si afferma quanto segue:

The Law of Demeter for functions requires that a method m of an object O may only invoke the methods of the following kinds of objects:[2]

1. O itself
2. m's parameters
3. Any objects created/instantiated within m
4. O's direct component objects
5. A global variable, accessible by O, in the scope of m

Una delle conseguenze di questa legge è che dovresti evitare di invocare metodi di oggetti restituiti da altri metodi in una lunga stringa punteggiata, come mostrato nel secondo esempio sopra:

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

È veramente difficile capire cosa stia facendo. Per capirlo devi capire cosa fa ogni procedura e quindi decifrare quale funzione viene chiamata su quale oggetto. Il primo blocco di codice

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

mostra chiaramente cosa stai cercando di fare - stai ricevendo un oggetto PowerManager, ottenendo un oggetto WakeLock da PowerManager, quindi acquisendo il wakeLock. Quanto sopra segue la regola 3 di LoD - stai istanziando questi oggetti all'interno del tuo codice, e quindi puoi utilizzarli per ottenere ciò che ti serve.

Forse un altro modo di pensare è ricordare che quando si crea un software si dovrebbe scrivere per chiarezza, non per brevità. Il 90% di tutto lo sviluppo del software è la manutenzione. Non scrivere mai un codice che non saresti felice di mantenere.

Buona fortuna.

Una cosa da notare è che il codice viene letto frequentemente, a diverse "profondità". Questo codice:

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

è facile da "sfiorare". Sono 3 dichiarazioni. Per prima cosa abbiamo un PowerManager . Quindi arriviamo a WakeLock . Quindi acquire wakeLock . Lo capisco molto facilmente osservando l'inizio di ogni riga; semplici assegnazioni di variabili sono davvero facili da riconoscere in parte come "Tipo varName = ..." e solo mentalmente sfiorano il "...". Allo stesso modo l'ultima affermazione non è ovviamente la forma del compito, ma coinvolge solo due nomi, quindi la "sintesi principale" è immediatamente evidente. Spesso è tutto ciò che avrei bisogno di sapere se stavo solo cercando di rispondere "cosa fa questo codice?" ad un livello elevato.

Se sto inseguendo un bug sottile che penso sia qui, ovviamente dovrò approfondire questo aspetto in modo molto più dettagliato, e in realtà ricorderò i "..." s. Ma la struttura dell'istruzione separata mi aiuta ancora a fare quell'unica affermazione alla volta (particolarmente utile se devo approfondire l'implementazione delle cose che vengono chiamate in ogni affermazione, quando torno indietro ho capito perfettamente "una sola unità" e può quindi passare alla successiva dichiarazione).

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

Ora è tutta una dichiarazione. La struttura di livello superiore non è così facile da leggere; nella versione originale dell'OP, senza interruzioni di riga e rientranza per comunicare visivamente qualsiasi struttura, avrei dovuto contare le parentesi per decodificarlo in una sequenza di 3 passaggi. Se alcune delle espressioni multiparte sono nidificate l'una dentro l'altra piuttosto che disposte come una catena di chiamate di metodo, allora potrebbe ancora apparire simile a questa, quindi devo fare attenzione a fidarmi di ciò senza contare le parentesi. E se mi fido della rientranza e sfiora l'ultima cosa come il punto presunto di tutto ciò, cosa mi dice .acquire() da solo?

A volte però, può essere quello che vuoi. Se applico la tua trasformazione a metà strada e ho scritto:

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

Ora questo comunica con una rapida panoramica "ottieni un WakeLock , poi acquire it". Ancora più semplice della prima versione. È immediatamente evidente che la cosa acquisita è una WakeLock . Se ottenere il PowerManager è solo un sub-dettaglio che è alquanto insignificante al punto di questo codice, ma wakeLock è importante, quindi potrebbe effettivamente aiutare a seppellire il PowerManager roba quindi è naturale scorrerlo sopra se stai solo cercando di farti un'idea di cosa fa questo codice. E non nominare comunica che è usato solo una volta, e qualche volta questo è ciò che è importante (se il resto dello scope è molto lungo, dovrei leggere tutto ciò che per dire se è mai usato di nuovo, anche se l'utilizzo di sotto-ambiti espliciti può essere un altro modo per affrontarlo, se la lingua li supporta).

Il che dimostra che tutto dipende dal contesto e da ciò che si desidera comunicare. Come per la scrittura di prosa in linguaggio naturale, ci sono sempre molti modi per scrivere un dato pezzo di codice che sono sostanzialmente equivalenti in termini di contenuto informativo. Come con la scrittura di prosa in linguaggio naturale, in genere la scelta tra loro dovrebbe non essere applicare regole meccanicistiche come "eliminare qualsiasi variabile locale che si verifica una volta sola". Piuttosto come scegli di scrivere il tuo codice metterà in risalto alcune cose e de-enfatizzerà gli altri. Dovresti sforzarti di fare consapevolmente queste scelte (inclusa la scelta di scrivere codice meno leggibile per motivi tecnici, a volte), in base a ciò che in realtà vuoi sottolineare. Pensa soprattutto a ciò che servirà ai lettori che hanno solo bisogno di "ottenere il succo" del tuo codice (a vari livelli), poiché ciò avverrà molto più spesso di una lettura di espressioni per espressione molto ravvicinata.

Questo è persino un antipattern con il suo nome: Train Wreck . Diversi motivi per evitare la sostituzione sono già stati dichiarati:

• Più difficile da leggere
• Più difficile da eseguire il debug (sia per visualizzare le variabili che rilevare le posizioni delle eccezioni)
• Violazione della legge di Demetra (LoD)

Considera se questo metodo conosce troppo gli altri oggetti. Il metodo di concatenamento è un'alternativa che potrebbe aiutarti a ridurre l'accoppiamento.

Ricorda anche che i riferimenti agli oggetti sono davvero economici.

La domanda dichiarata è "Dovremmo eliminare le variabili locali se possiamo"?

No, non dovresti eliminarlo solo perché puoi.

Dovresti seguire le linee guida sulla codifica nel tuo negozio.

Per la maggior parte le variabili locali rendono il codice più facile da leggere ed eseguire il debug.

Mi piace quello che hai fatto con PowerManager powerManager . Per me una minuscola della classe significa che è solo una volta.

Quando non dovresti, è se la variabile trattiene risorse costose. Molte lingue hanno sintassi per una variabile locale che deve essere pulita / rilasciata. In C # sta usando.

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

Un altro aspetto importante non è stato menzionato nelle altre risposte: ogni volta che aggiungi una variabile, introduci lo stato mutabile. Questo di solito è una cosa negativa perché rende il codice più complesso e quindi più difficile da capire e da testare. Naturalmente, minore è lo scopo della variabile, minore è il problema.

Quello che vuoi in realtà non è una variabile il cui valore può essere modificato ma una costante temporanea. Quindi considera di esprimere questo nel tuo codice se la tua lingua lo consente. In Java puoi usare final e in C ++ puoi usare const . Nella maggior parte dei linguaggi funzionali, questo è il comportamento predefinito.

È vero che le variabili locali sono il tipo meno dannoso di stato mutabile. Le variabili membro possono causare molti più problemi e le variabili statiche sono anche peggio. Tuttavia, ritengo importante esprimere nel modo più preciso possibile il codice che cosa dovrebbe fare. E c'è un'enorme differenza tra una variabile che potrebbe essere modificata in seguito e un semplice nome per un risultato intermedio. Quindi se la tua lingua ti consente di esprimere questa differenza, fallo e basta.

Per me nessuno di questi è più difficile da leggere, a patto che indentiate correttamente. Se l'uso di una variabile intermedia ha reso le cose più chiare, il Pattern Builder non esisterebbe.

Ovviamente il ruolo principale del modello di builder non è quello di eliminare la variabile intermedia, ma di assemblare in modo fluido una variabile complessa. Ma fa cadere la variabile intermedia nel modo.

I problemi con le variabili intermedie sono i seguenti:

1. Assegnale loro un nome
2. Quando leggerò il tuo codice vedrò una variabile locale dichiarata qui. Inizierò a chiedermi se si tratta di una variabile intermedia o che verrà utilizzata da qualche altra parte.

Tuttavia, c'è un vantaggio interessante: puoi controllare i risultati dell'intermediario.

Se pensi che essere in grado di guardare lo stato della variabile intermedia quando il debug è interessante, allora usane uno. Questo è praticamente l'unico motivo per cui utilizzo le variabili intermedie.