¿Razonamiento para preferir variables locales sobre variables de instancia?

109

La base de código en la que estoy trabajando con frecuencia usa variables de instancia para compartir datos entre varios métodos triviales. El desarrollador original insiste en que esto se adhiere a las mejores prácticas establecidas en el libro Clean Code del tío Bob / Robert Martin: "La primera regla de funciones es que deberían ser pequeñas". y "El número ideal de argumentos para una función es cero (niládico). (...) Los argumentos son difíciles. Toman mucho poder conceptual".

Un ejemplo:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  private byte[] encodedData;
  private EncryptionInfo encryptionInfo;
  private EncryptedObject payloadOfResponse;
  private URI destinationURI;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    getEncodedData(encryptedRequest);
    getEncryptionInfo();
    getDestinationURI();
    passRequestToServiceClient();

    return cryptoService.encryptResponse(payloadOfResponse);
  }

  private void getEncodedData(EncryptedRequest encryptedRequest) {
    encodedData = cryptoService.decryptRequest(encryptedRequest, byte[].class);
  }

  private void getEncryptionInfo() {
    encryptionInfo = cryptoService.getEncryptionInfoForDefaultClient();
  }

  private void getDestinationURI() {
    destinationURI = router.getDestination().getUri();
  }

  private void passRequestToServiceClient() {
    payloadOfResponse = serviceClient.handle(destinationURI, encodedData, encryptionInfo);
  }
}

Refactorizaría eso en lo siguiente usando variables locales:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    byte[] encodedData = cryptoService.decryptRequest(encryptedRequest, byte[].class);
    EncryptionInfo encryptionInfo = cryptoService.getEncryptionInfoForDefaultClient();
    URI destinationURI = router.getDestination().getUri();
    EncryptedObject payloadOfResponse = serviceClient.handle(destinationURI, encodedData,
      encryptionInfo);

    return cryptoService.encryptResponse(payloadOfResponse);
  }
}

Esto es más corto, elimina el acoplamiento de datos implícito entre los diversos métodos triviales y limita los alcances variables al mínimo requerido. Sin embargo, a pesar de estos beneficios, todavía no puedo convencer al desarrollador original de que esta refactorización está justificada, ya que parece contradecir las prácticas del tío Bob mencionadas anteriormente.

De ahí mis preguntas: ¿Cuál es la razón objetiva y científica para favorecer las variables locales sobre las variables de instancia? Parece que no puedo señalarlo. Mi intuición me dice que los acoplamientos ocultos son malos y que un alcance estrecho es mejor que uno amplio. Pero, ¿cuál es la ciencia para respaldar esto?

Y a la inversa, ¿hay alguna desventaja en esta refactorización que posiblemente haya pasado por alto?

Alejandro
fuente
Los comentarios no son para discusión extendida; Esta conversación se ha movido al chat .
maple_shaft

Respuestas:

170

¿Cuál es la razón objetiva y científica para favorecer las variables locales sobre las variables de instancia?

El alcance no es un estado binario, es un gradiente. Puede clasificarlos de mayor a menor:

Global > Class > Local (method) > Local (code block, e.g. if, for, ...)

Editar: lo que yo llamo un "alcance de clase" es lo que quiere decir con "variable de instancia". Que yo sepa, esos son sinónimos, pero soy un desarrollador de C #, no un desarrollador de Java. En aras de la brevedad, he agrupado todas las estadísticas en la categoría global, ya que las estadísticas no son el tema de la pregunta.

Cuanto más pequeño sea el alcance, mejor. La razón es que las variables deberían vivir en el menor alcance posible . Hay muchos beneficios para esto:

  • Te obliga a pensar en la responsabilidad de la clase actual y te ayuda a cumplir con SRP.
  • Le permite no tiene que evitar conflictos de nombres globales, por ejemplo, si dos o más clases tienen una Namepropiedad, usted no está obligado a ellos como prefijo FooName, BarName, ... Por lo tanto mantener sus nombres de variables tan limpia y concisa posible.
  • Disminuye el código limitando las variables disponibles (por ejemplo, para Intellisense) a aquellas que son contextualmente relevantes.
  • Permite alguna forma de control de acceso para que sus datos no puedan ser manipulados por algún actor que no conozca (por ejemplo, una clase diferente desarrollada por un colega).
  • Hace que el código sea más legible a medida que se asegura de que la declaración de estas variables intente mantenerse lo más cerca posible del uso real de estas variables.
  • Declarar las variables de manera arbitraria en un ámbito excesivamente amplio a menudo es indicativo de un desarrollador que no comprende la POO o cómo implementarla. Ver variables con un alcance excesivamente amplio actúa como una señal de alerta de que probablemente haya algo mal con el enfoque OOP (ya sea con el desarrollador en general o la base de código en específico).
  • (Comentario de Kevin) El uso de locales te obliga a hacer las cosas en el orden correcto. En el código original (variable de clase), podría moverse incorrectamente passRequestToServiceClient()a la parte superior del método, y aún así se compilaría. Con los locales, solo podría cometer ese error si pasa una variable no inicializada, que es bastante obvio que en realidad no lo haga.

Sin embargo, a pesar de estos beneficios, todavía no puedo convencer al desarrollador original de que esta refactorización está justificada, ya que parece contradecir las prácticas del tío Bob mencionadas anteriormente.

Y a la inversa, ¿hay alguna desventaja en esta refactorización que posiblemente haya pasado por alto?

El problema aquí es que su argumento para las variables locales es válido, pero también ha realizado cambios adicionales que no son correctos y hacen que su solución sugerida falle la prueba de olor.

Si bien entiendo su sugerencia de "no hay variable de clase" y tiene mérito, en realidad también ha eliminado los métodos en sí mismos, y ese es un juego de pelota completamente diferente. Los métodos deberían haberse quedado, y en su lugar debería haberlos alterado para devolver su valor en lugar de almacenarlo en una variable de clase:

private byte[] getEncodedData() {
    return cryptoService.decryptRequest(encryptedRequest, byte[].class);
}

private EncryptionInfo getEncryptionInfo() {
    return cryptoService.getEncryptionInfoForDefaultClient();
}

// and so on...

Estoy de acuerdo con lo que has hecho en el processmétodo, pero deberías haber llamado a los submétodos privados en lugar de ejecutar sus cuerpos directamente.

public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    byte[] encodedData = getEncodedData();
    EncryptionInfo encryptionInfo = getEncryptionInfo();

    //and so on...

    return cryptoService.encryptResponse(payloadOfResponse);
}

Desea esa capa adicional de abstracción, especialmente cuando se encuentra con métodos que deben reutilizarse varias veces. Incluso si actualmente no reutiliza sus métodos , sigue siendo una buena práctica crear submétodos donde sea relevante, aunque solo sea para ayudar a la legibilidad del código.

Independientemente del argumento de la variable local, noté de inmediato que su solución sugerida es considerablemente menos legible que la original. Admito que el uso desenfrenado de las variables de clase también resta valor a la legibilidad del código, pero no a primera vista en comparación con el hecho de que haya apilado toda la lógica en un solo método (ahora de largo aliento).

Flater
fuente
Los comentarios no son para discusión extendida; Esta conversación se ha movido al chat .
maple_shaft
79

El código original está usando variables miembro como argumentos. Cuando dice minimizar el número de argumentos, lo que realmente quiere decir es minimizar la cantidad de datos que requieren los métodos para funcionar. Poner esos datos en variables miembro no mejora nada.

Alex
fuente
20
¡Totalmente de acuerdo! Esas variables miembro son simplemente argumentos de funciones implícitas. De hecho, es peor ya que ahora no hay un vínculo explícito entre el valor de esas variables y el uso de la función (desde un POV externo)
Rémi
1
Yo diría que esto no es lo que significa el libro. ¿Cuántas funciones requieren exactamente cero datos de entrada para ejecutarse? Esta es una de las partes que creo que el libro se equivocó.
Qwertie
1
@Qwertie Si tiene un objeto, los datos en los que trabaja pueden estar completamente encapsulados dentro de él. Funciones como process.Start();o myString.ToLowerCase()no deberían parecer demasiado extrañas (y de hecho son las más fáciles de entender).
R. Schmitz
55
Ambos tienen un argumento: el implícito this. Incluso se podría argumentar que este argumento se da explícitamente antes del punto.
BlackJack
47

Otras respuestas ya explicaron perfectamente los beneficios de las variables locales, por lo que todo lo que queda es esta parte de su pregunta:

Sin embargo, a pesar de estos beneficios, todavía no puedo convencer al desarrollador original de que esta refactorización está justificada, ya que parece contradecir las prácticas del tío Bob mencionadas anteriormente.

Eso debería ser fácil. Simplemente apúntelo a la siguiente cita en el Código Limpio del Tío Bob:

No tiene efectos secundarios

Los efectos secundarios son mentiras. Su función promete hacer una cosa, pero también hace otras cosas ocultas. Algunas veces hará cambios inesperados a las variables de su propia clase. A veces los hará pasar a los parámetros pasados ​​a la función o al sistema global. En cualquier caso, son falsedades engañosas y dañinas que a menudo resultan en acoplamientos temporales extraños y dependencias de orden.

(ejemplo omitido)

Este efecto secundario crea un acoplamiento temporal. Es decir, checkPassword solo se puede llamar en ciertos momentos (en otras palabras, cuando es seguro inicializar la sesión). Si se llama fuera de servicio, los datos de la sesión pueden perderse inadvertidamente. Los acoplamientos temporales son confusos, especialmente cuando están ocultos como un efecto secundario. Si debe tener un acoplamiento temporal, debe dejarlo claro en el nombre de la función. En este caso, podríamos cambiar el nombre de la función checkPasswordAndInitializeSession, aunque eso ciertamente viola "Hacer una cosa".

Es decir, el tío Bob no solo dice que una función debe tener pocos argumentos, también dice que las funciones deben evitar interactuar con el estado no local siempre que sea posible.

Meriton
fuente
3
En un "mundo perfecto", esta sería la segunda respuesta en la lista. La primera respuesta para la situación ideal que el compañero de trabajo escucha razonar, pero si el compañero de trabajo es un fanático, esta respuesta aquí trataría la situación sin demasiada confusión.
R. Schmitz
2
Para una variación más pragmática de esta idea, es simplemente que es mucho más fácil razonar sobre el estado local que la instancia o el estado global. La mutabilidad y los efectos secundarios bien definidos y estrictamente contenidos rara vez conducen a problemas. Por ejemplo, muchas funciones de ordenación operan in situ a través de efectos secundarios, pero es fácil razonar en un ámbito local.
Beefster
1
Ah, el viejo "en un axiomático contradictorio, es posible probar cualquier cosa". Como no hay verdades duras y mentiras IRL, cualquier dogma tendrá que incluir declaraciones que expresen cosas opuestas, es decir, ser contradictorias.
ivan_pozdeev
26

"Contradice lo que piensa el tío de alguien" NUNCA es un buen argumento. NUNCA. No tomes la sabiduría de los tíos, piensa por ti mismo.

Dicho esto, las variables de instancia deben usarse para almacenar información que en realidad se requiere que se almacene de forma permanente o semipermanente. La información aquí no es. Es muy simple vivir sin las variables de instancia, por lo que pueden ir.

Prueba: escriba un comentario de documentación para cada variable de instancia. ¿Puedes escribir algo que no sea completamente inútil? Y escriba un comentario de documentación a los cuatro accesores. Son igualmente inútiles.

Lo peor es asumir la forma de descifrar los cambios, ya que utiliza un servicio de cifrado diferente. En lugar de tener que cambiar cuatro líneas de código, debe reemplazar cuatro variables de instancia con diferentes, cuatro captadores con diferentes y cambiar cuatro líneas de código.

Pero, por supuesto, la primera versión es preferible si le pagan por la línea de código. 31 líneas en lugar de 11 líneas. Tres veces más líneas para escribir y mantener para siempre, para leer cuando está depurando algo, para adaptarse cuando se necesitan cambios, para duplicar si admite un segundo servicio de cifrado.

(Se perdió el punto importante de que el uso de variables locales lo obliga a realizar las llamadas en el orden correcto).

gnasher729
fuente
16
Pensar por ti mismo es, por supuesto, bueno. Pero su párrafo inicial incluye efectivamente a los alumnos o juniors que descartan los aportes de los maestros o seniors; que va demasiado lejos
Flater
99
@Flater Después de pensar en el aporte de los maestros o personas de la tercera edad, y ver que están equivocados, descartar su aporte es lo único correcto. Al final, no se trata de descartar, sino de cuestionarlo y solo descartarlo si definitivamente demuestra que está equivocado.
glglgl
10
@ChristianHackl: Estoy totalmente de acuerdo con no seguir ciegamente el tradicionalismo, pero tampoco lo descartaría ciegamente. Aparentemente, la respuesta sugiere evitar el conocimiento adquirido en favor de su propio punto de vista, y ese no es un enfoque saludable. Seguir ciegamente las opiniones de los demás, no. Cuestionar algo cuando no estás de acuerdo, obviamente sí. Lo descartó por completo porque no está de acuerdo, no. Mientras lo leo, el primer párrafo de la respuesta al menos parece sugerir el último. Depende mucho de lo que significa Gnasher con "piensa por ti mismo", que necesita elaboración.
Flater
99
En una plataforma de intercambio de sabiduría, esto parece un poco fuera de lugar ...
drjpizzle
44
NUNCA también es NUNCA un buen argumento ... Estoy totalmente de acuerdo en que existen reglas para guiar, no para prescribir. Sin embargo, las reglas sobre reglas no son más que una subclase de reglas, por lo que pronunció su propio veredicto ...
cmaster
14

¿Cuál es la razón objetiva y científica para favorecer las variables locales sobre las variables de instancia? Parece que no puedo señalarlo. Mi intuición me dice que los acoplamientos ocultos son malos y que un alcance estrecho es mejor que uno amplio. Pero, ¿cuál es la ciencia para respaldar esto?

Las variables de instancia son para representar las propiedades de su objeto host, no para representar propiedades específicas de subprocesos de cómputo con un alcance más limitado que el objeto mismo. Algunas de las razones para establecer tal distinción que parecen no haberse cubierto ya giran en torno a la concurrencia y la reentrada. Si los métodos intercambian datos estableciendo los valores de las variables de instancia, entonces dos subprocesos concurrentes pueden cambiar fácilmente los valores de cada uno para esas variables de instancia, produciendo errores intermitentes y difíciles de encontrar.

Incluso un solo hilo puede encontrar problemas en ese sentido, porque existe un alto riesgo de que un patrón de intercambio de datos que se base en variables de instancia haga que los métodos no sean reentrantes. Del mismo modo, si se usan las mismas variables para transmitir datos entre diferentes pares de métodos, existe el riesgo de que un solo hilo que ejecute incluso una cadena no recursiva de invocaciones de métodos se encuentre con errores que giran en torno a modificaciones inesperadas de las variables de instancia involucradas.

Para obtener resultados correctos confiables en tal escenario, debe usar variables separadas para comunicarse entre cada par de métodos donde uno invoca al otro, o bien para que cada implementación del método tenga en cuenta todos los detalles de implementación de todos los demás métodos que invoca, ya sea directa o indirectamente. Esto es frágil y se escala mal.

John Bollinger
fuente
77
Hasta ahora, esta es la única respuesta que menciona la seguridad del hilo y la concurrencia. Eso es sorprendente dado el ejemplo de código específico en la pregunta: una instancia de SomeBusinessProcess no puede procesar de manera segura múltiples solicitudes encriptadas a la vez. El método public EncryptedResponse process(EncryptedRequest encryptedRequest)no está sincronizado, y las llamadas simultáneas posiblemente bloquearían los valores de las variables de instancia. Este es un buen punto para mencionar.
Joshua Taylor
9

Discutiendo simplemente process(...), el ejemplo de sus colegas es mucho más legible en el sentido de la lógica empresarial. Por el contrario, su contraejemplo requiere más que una mirada superficial para extraer cualquier significado.

Dicho esto, el código limpio es legible y de buena calidad: empujar al estado local a un espacio más global es solo un ensamblaje de alto nivel, por lo que un cero para la calidad.

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest request) {
    checkNotNull(encryptedRequest);

    return encryptResponse
      (routeTo
         ( destination()
         , requestData(request)
         , destinationEncryption()
         )
      );
  }

  private byte[] requestData(EncryptedRequest encryptedRequest) {
    return cryptoService.decryptRequest(encryptedRequest, byte[].class);
  }

  private EncryptionInfo destinationEncryption() {
    return cryptoService.getEncryptionInfoForDefaultClient();
  }

  private URI destination() {
    return router.getDestination().getUri();
  }

  private EncryptedObject routeTo(URI destinationURI, byte[] encodedData, EncryptionInfo encryptionInfo) {
    return serviceClient.handle(destinationURI, encodedData, encryptionInfo);
  }

  private void encryptResponse(EncryptedObject payloadOfResponse) {
    return cryptoService.encryptResponse(payloadOfResponse);
  }
}

Esta es una interpretación que elimina la necesidad de variables en cualquier ámbito. Sí, el compilador los generará, pero la parte importante es que controla eso para que el código sea eficiente. Mientras que también es relativamente legible.

Solo un punto para nombrar. Desea el nombre más corto que sea significativo y amplíe la información ya disponible. es decir. destinationURI, el 'URI' ya es conocido por la firma de tipo.

Kain0_0
fuente
44
Eliminar todas las variables no necesariamente hace que el código sea más fácil de leer.
Pharap
Elimine todas las variables completamente con un estilo sin puntos en.wikipedia.org/wiki/Tacit_programming
Marcin
@Pharap Verdadero, la falta de variables no garantiza la legibilidad. En algunos casos, incluso dificulta la depuración. El punto es que los nombres bien elegidos, un uso claro de la expresión, pueden comunicar una idea muy claramente, sin dejar de ser eficiente.
Kain0_0
7

Simplemente eliminaría estas variables y métodos privados por completo. Aquí está mi refactor:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    return cryptoService.encryptResponse(
        serviceClient.handle(router.getDestination().getUri(),
        cryptoService.decryptRequest(encryptedRequest, byte[].class),
        cryptoService.getEncryptionInfoForDefaultClient()));
  }
}

Para el método privado, por ejemplo, router.getDestination().getUri()es más claro y más legible que getDestinationURI(). Incluso repetiría eso si uso la misma línea dos veces en la misma clase. Para verlo de otra manera, si es necesario getDestinationURI(), entonces probablemente pertenezca a otra clase, no a la SomeBusinessProcessclase.

Para las variables y propiedades, la necesidad común de ellas es mantener los valores que se utilizarán más adelante en el tiempo. Si la clase no tiene una interfaz pública para las propiedades, probablemente no deberían ser propiedades. El peor tipo de uso de propiedades de clase es probablemente para pasar valores entre métodos privados por medio de efectos secundarios.

De todos modos, la clase solo necesita hacer process()y luego el objeto será desechado, no hay necesidad de mantener ningún estado en la memoria. Otro potencial refactorial sería eliminar el CryptoService de esa clase.

Con base en los comentarios, quiero agregar que esta respuesta se basa en la práctica del mundo real. De hecho, en la revisión de código, lo primero que elegiría es refactorizar la clase y mover el trabajo de cifrado / descifrado. Una vez hecho esto, preguntaría si se necesitan los métodos y las variables, si se nombran correctamente, etc. El código final probablemente estará más cerca de esto:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;

  public Response process(Request request) {
    return serviceClient.handle(router.getDestination().getUri());
  }
}

Con el código anterior, no creo que necesite más refactorización. Al igual que con las reglas, creo que se necesita experiencia para saber cuándo y cuándo no aplicarlas. Las reglas no son teorías probadas para funcionar en todas las situaciones.

La revisión del código, por otro lado, tiene un impacto real en el tiempo que puede pasar un fragmento de código. Mi truco es tener menos código y facilitar su comprensión. Un nombre de variable puede ser un punto de discusión, si puedo eliminarlo, los revisores ni siquiera tendrían que pensarlo.

imel96
fuente
Mi voto a favor, aunque muchos dudarán aquí. Por supuesto, algunas abstracciones tienen sentido. (Por cierto, un método llamado "proceso" es terrible). Pero aquí la lógica es mínima. Sin embargo, la pregunta del OP es sobre un estilo de código completo, y allí el caso puede ser más complejo.
Joop Eggen
1
Un problema claro al encadenarlo todo en una llamada de método es la legibilidad total. Tampoco funciona si necesita más de una operación en un objeto determinado. Además, esto es casi imposible de depurar porque no puede pasar por las operaciones e inspeccionar los objetos. Si bien esto funciona a nivel técnico, no recomendaría esto, ya que descuida masivamente los aspectos no relacionados con el tiempo de ejecución del desarrollo de software.
Flater
@Flater Estoy de acuerdo con sus comentarios, no queremos aplicar esto en todas partes. Edité mi respuesta para aclarar mi posición práctica. Lo que quiero mostrar es que en la práctica solo aplicamos reglas cuando es apropiado. La llamada al método de encadenamiento está bien en este caso, y si necesito depurar, invocaría las pruebas para los métodos encadenados.
imel96
@JoopEggen Sí, las abstracciones tienen sentido. En el ejemplo, los métodos privados no dan ninguna abstracción de todos modos, los usuarios de la clase ni siquiera saben sobre ellos
imel96
1
@ imel96 es divertido que probablemente seas una de las pocas personas aquí que notó que el acoplamiento entre ServiceClient y CryptoService hace que valga la pena enfocarse en inyectar CS en SC en lugar de SBP, resolviendo el problema subyacente a un nivel arquitectónico más alto ... ese es IMVHO, el punto principal de esta historia; es muy fácil hacer un seguimiento del panorama general mientras se enfoca en los detalles.
vaxquis
4

La respuesta de Flater cubre cuestiones de alcance bastante bien, pero creo que aquí también hay otro problema.

Tenga en cuenta que hay una diferencia entre una función que procesa datos y una función que simplemente accede a los datos .

El primero ejecuta la lógica comercial real, mientras que el segundo guarda la escritura y quizás agrega seguridad al agregar una interfaz más simple y reutilizable.

En este caso, parece que las funciones de acceso a datos no guardan la escritura y no se reutilizan en ningún lugar (o habría otros problemas al eliminarlas). Entonces estas funciones simplemente no deberían existir.

Al mantener solo la lógica de negocios en funciones con nombre, obtenemos lo mejor de ambos mundos (en algún lugar entre la respuesta de Flater y la respuesta de imel96 ):

public EncryptedResponse process(EncryptedRequest encryptedRequest) {

    byte[] requestData = decryptRequest(encryptedRequest);
    EncryptedObject responseData = handleRequest(router.getDestination().getUri(), requestData, cryptoService.getEncryptionInfoForDefaultClient());
    EncryptedResponse response = encryptResponse(responseData);

    return response;
}

// define: decryptRequest(), handleRequest(), encryptResponse()
usuario673679
fuente
3

Lo primero y más importante: el tío Bob parece ser como un predicador a veces, pero afirma que hay excepciones a sus reglas.

La idea de Clean Code es mejorar la legibilidad y evitar errores. Hay varias reglas que se están violando entre sí.

Su argumento sobre las funciones es que las funciones niládicas son las mejores, sin embargo, hasta tres parámetros son aceptables. Personalmente, creo que 4 también están bien.

Cuando se usan variables de instancia, deben formar una clase coherente. Eso significa que las variables deben usarse en muchos, si no en todos los métodos no estáticos.

Las variables que no se usan en muchos lugares de la clase, se deben mover.

No consideraría óptima la versión original ni la refactorizada, y @Flater ya declaró muy bien qué se puede hacer con los valores de retorno. Mejora la legibilidad y reduce los errores al usar valores de retorno.

kap
fuente
1

Las variables locales reducen el alcance, por lo tanto, limitan las formas en que se pueden usar las variables y, por lo tanto, ayudan a prevenir ciertas clases de error y mejoran la legibilidad.

La variable de instancia reduce las formas en que se puede llamar a la función, lo que también ayuda a reducir ciertas clases de error y mejora la legibilidad.

Decir que uno está bien y el otro está mal puede ser una conclusión válida en cualquier caso en particular, pero como consejo general ...

TL; DR: Creo que la razón por la que hueles demasiado celo es porque hay demasiado celo.

drjpizzle
fuente
0

A pesar de que los métodos que comienzan con get ... no deberían volver nulos, la separación de los niveles de abstracciones dentro de los métodos se da en la primera solución. Aunque la segunda solución tiene un alcance más amplio, aún es más difícil razonar sobre lo que está sucediendo en el método. Las asignaciones de variables locales no son necesarias aquí. Mantendría los nombres de los métodos y refactorizaría el código a algo así:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    return getEncryptedResponse(
            passRequestToServiceClient(getDestinationURI(), getEncodedData(encryptedRequest) getEncryptionInfo())
        );
  }

  private EncryptedResponse getEncryptedResponse(EncryptedObject encryptedObject) {
    return cryptoService.encryptResponse(encryptedObject);
  }

  private byte[] getEncodedData(EncryptedRequest encryptedRequest) {
    return cryptoService.decryptRequest(encryptedRequest, byte[].class);
  }

  private EncryptionInfo getEncryptionInfo() {
    return cryptoService.getEncryptionInfoForDefaultClient();
  }

  private URI getDestinationURI() {
    return router.getDestination().getUri();
  }

  private EncryptedObject passRequestToServiceClient(URI destinationURI, byte[] encodedData, EncryptionInfo encryptionInfo) {
    return serviceClient.handle(destinationURI, encodedData, encryptionInfo);
  }
}
Roman Weis
fuente
0

Ambas cosas hacen lo mismo y las diferencias de rendimiento son imperceptibles, por lo que no creo que haya un argumento científico . Todo se reduce a preferencias subjetivas entonces.

Y a mí también me gusta más tu manera que la de tu colega. ¿Por qué? Porque creo que es más fácil de leer y entender, a pesar de lo que dice un autor de libros.

Ambas formas logran lo mismo, pero su camino está más extendido. Para leer ese código, debe alternar entre varias funciones y variables miembro. No todo está condensado en un solo lugar, debes recordar todo eso en tu cabeza para entenderlo. Es una carga cognitiva mucho mayor.

Por el contrario, su enfoque lo empaqueta todo mucho más densamente, pero no para hacerlo impenetrable. Simplemente lo lees línea por línea, y no necesitas memorizar tanto para entenderlo.

Sin embargo, si está acostumbrado a codificar que se presenten de tal manera, me imagino que para él podría ser al revés.

Vilx-
fuente
De hecho, eso ha demostrado ser un obstáculo importante para que entienda el flujo. Este ejemplo es bastante pequeño, pero otro utilizó 35 (!) Variables de instancia para sus resultados intermedios, sin contar la docena de variables de instancia que contienen las dependencias. Fue bastante difícil de seguir, ya que tienes que hacer un seguimiento de lo que ya se ha establecido anteriormente. Algunos incluso se reutilizaron más tarde, lo que lo hizo aún más difícil. Afortunadamente, los argumentos presentados aquí finalmente han convencido a mi colega de estar de acuerdo con la refactorización.
Alexander