L'uso di blocchi di scope interni in uno stile di funzione non valido?

10

Ci sono alcuni casi (abbastanza rari) in cui esiste il rischio di:

  • riutilizzo di una variabile che non è destinata a essere riutilizzata (vedi esempio 1),

  • o utilizzando una variabile anziché un'altra, semanticamente chiusa (vedi esempio 2).

Esempio 1:

var data = this.InitializeData();
if (this.IsConsistent(data, this.state))
{
    this.ETL.Process(data); // Alters original data in a way it couldn't be used any longer.
}

// ...

foreach (var flow in data.Flows)
{
    // This shouldn't happen: given that ETL possibly altered the contents of 'data', it is
    // not longer reliable to use 'data.Flows'.
}

Esempio 2:

var userSettingsFile = SettingsFiles.LoadForUser();
var appSettingsFile = SettingsFiles.LoadForApp();

if (someCondition)
{
    userSettingsFile.Destroy();
}

userSettingsFile.ParseAndApply(); // There is a mistake here: 'userSettingsFile' was maybe
                                  // destroyed. It's 'appSettingsFile' which should have
                                  // been used instead.

Questo rischio può essere mitigato introducendo un ambito:

Esempio 1:

// There is no 'foreach', 'if' or anything like this before '{'.

{
    var data = this.InitializeData();
    if (this.IsConsistent(data, this.state))
    {
        this.ETL.Process(data);
    }
}

// ...

// A few lines later, we can't use 'data.Flows', because it doesn't exist in this scope.

Esempio 2:

{
    var userSettingsFile = SettingsFiles.LoadForUser();

    if (someCondition)
    {
        userSettingsFile.Destroy();
    }
}

{
    var appSettingsFile = SettingsFiles.LoadForApp();

    // 'userSettingsFile' is out of scope. There is no risk to use it instead of
    // 'appSettingsFile'.
}

Sembra sbagliato? Eviteresti tale sintassi? È difficile da capire per i principianti?

    
posta Arseni Mourzenko 06.06.2013 - 21:55
fonte

4 risposte

33

Se la tua funzione è così lunga da non poter più riconoscere effetti secondari indesiderati o riutilizzo illegale delle variabili, è giunto il momento di suddividerla in funzioni più piccole, il che rende un ambito interno inutile.

Per avvalorare questa esperienza personale: alcuni anni fa ho ereditato un progetto legacy C ++ con ~ 150K linee di codice, e conteneva alcuni metodi usando esattamente questa tecnica. E indovina un po '- tutti quei metodi erano troppo lunghi. Come abbiamo rifattorizzato la maggior parte di quel codice, i metodi sono diventati sempre più piccoli, e sono abbastanza sicuro che non ci siano più metodi "interni" rimanenti; semplicemente non sono necessari.

    
risposta data 06.06.2013 - 22:08
fonte
3

È molto più facile pensare a un principiante (e qualsiasi programmatore):

  • non utilizza una variabile non pertinente

che pensare a:

  • aggiungere strutture di codice che mirano a evitare di non utilizzare una variabile che non è pertinente.

Inoltre, dal punto di vista di un lettore, tali blocchi non hanno alcun ruolo apparente: rimuoverli non ha alcun impatto sull'esecuzione. Il lettore potrebbe grattarsi la testa cercando di indovinare ciò che il programmatore voleva ottenere.

    
risposta data 07.06.2013 - 09:56
fonte
2

L'utilizzo di un oscilloscopio è tecnicamente corretto. Ma se vuoi rendere il tuo codice più leggibile, devi estrarre quella parte in un altro metodo e assegnargli un nome completo di significato. In questo modo puoi dare una portata delle tue variabili e anche dare un nome al tuo compito.

    
risposta data 16.11.2015 - 04:29
fonte
0

Sono d'accordo sul fatto che il riutilizzo della variabile sia il più delle volte un odore di codice. Probabilmente tale codice dovrebbe essere rifattorizzato in blocchi più piccoli e autonomi.

C'è uno scenario particolare, OTOH, in C #, quando tendono a pop-up - cioè quando si utilizzano costrutti di case-switch. La situazione è spiegata molto bene in: dichiarazione switch C # con / senza parentesi graffe ... qual è la differenza?

    
risposta data 12.06.2013 - 11:48
fonte

Leggi altre domande sui tag