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

10

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?

Arseni Mourzenko
fuente
77
¿Podría argumentar que cada "bloque de alcance" independiente debería ser su propio método o función?
Dan Pichelman
Yo diría que "a menudo" no está tan bien implementado como podría estarlo (pero probablemente no es un problema de "diseño"). A veces, por diseño, es inevitable (por ejemplo, excepción / alcance de recursos).
JustinC

Respuestas:

35

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.

Doc Brown
fuente
44
¿Pero por qué es malo? Tener muchas funciones con nombre también es confuso.
Kris Van Bael
1
+1 Exactamente, si necesita un alcance, es probable que esté realizando una acción particular que se puede extraer a una función. Kris, no se trata de crear montones y montones de funciones, es que cuando ocurren estos casos (donde el alcance de un bloque puede entrar en conflicto con el de otro), generalmente se debería haber usado una función. Veo esto en otros idiomas (es decir, JavaScript, con funciones antiguas de IIFE en lugar de simples) todo el tiempo también.
Benjamin Gruenbaum
10
@KrisVanBael: "Tener muchas funciones con nombre también es confuso": si usa nombres descriptivos, todo lo contrario es cierto. La creación de funciones demasiado grandes es la razón principal por la que el código se vuelve difícil de mantener, y aún más difícil de ampliar.
Doc Brown
2
Si hay tantas funciones que nombrarlas claramente se convierte en un problema, posiblemente algunas de ellas se pueden incluir en un nuevo tipo, ya que pueden tener una independencia significativa de su cuerpo de funciones original. Algunos podrían estar tan estrechamente relacionados que podrían ser eliminados. De repente, no hay tantas funciones.
JustinC
3
Todavía no está convencido. A menudo, las funciones largas son realmente malas. Pero decir que cada necesidad de alcance requiere una función separada es demasiado blanco y negro para mí.
Kris Van Bael
3

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.

Mouviciel
fuente
2

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.

DesarrolladorArnab
fuente
0

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?

Dejan Stanič
fuente