¿Es el uso de bloques de alcance interno dentro de una función mal estilo?

Hay algunos casos (bastante raros) en los que existe el riesgo de:

• reutilizando una variable que no está destinada a ser reutilizada (ver ejemplo 1),

• o usando una variable en lugar de otra, semánticamente cerca (ver ejemplo 2).

Ejemplo 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`.
}

Ejemplo 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.

Este riesgo puede mitigarse introduciendo un alcance:

Ejemplo 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.

Ejemplo 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`.
}

¿Se ve mal? ¿Evitarías tal sintaxis? ¿Es difícil de entender por los principiantes?

Si su función es tan larga que ya no puede reconocer ningún efecto secundario no deseado o la reutilización ilegal de variables, entonces es hora de dividirla en funciones más pequeñas, lo que hace que un alcance interno no tenga sentido.

Para respaldar esto con alguna experiencia personal: hace algunos años heredé un proyecto heredado de C ++ con ~ 150K líneas de código, y contenía algunos métodos que usaban exactamente esta técnica. Y adivina qué: todos esos métodos fueron demasiado largos. A medida que reestructuramos la mayor parte de ese código, los métodos se hicieron cada vez más pequeños, y estoy bastante seguro de que ya no quedan métodos de "alcance interno"; simplemente no son necesarios.

Es mucho más fácil para un principiante (y cualquier programador) pensar en:

• no usar una variable que no es relevante

que pensar en:

• agregando estructuras de código con el objetivo de evitar que no use una variable que no sea relevante.

Además, desde el punto de vista del lector, tales bloques no tienen un papel aparente: eliminarlos no tiene ningún impacto en la ejecución. El lector puede rascarse la cabeza tratando de adivinar lo que el codificador quería lograr.

Usar un osciloscopio es técnicamente correcto. Pero si desea que su código sea más legible, debe extraer esa parte en otro método y darle un nombre completo. De esta manera, puede dar un alcance de sus variables y también dar un nombre a su tarea.

Estoy de acuerdo en que la reutilización de variables suele ser un olor a código. Probablemente dicho código debería refactorizarse en bloques más pequeños y autónomos.

Hay un escenario particular, OTOH, en C #, cuando tienden a aparecer, es decir, cuando se utilizan construcciones de mayúsculas y minúsculas. La situación se explica muy bien en: C # Cambiar la declaración con / sin llaves ... ¿cuál es la diferencia?