¿Es una buena práctica usar funciones solo para centralizar el código común?

20

Me encuentro con este problema mucho. Por ejemplo, actualmente escribo una función de lectura y una función de escritura, y ambas verifican si bufes un puntero NULL y si la modevariable está dentro de ciertos límites.

Esto es duplicación de código. Esto se puede resolver moviéndolo a su propia función. ¿Pero debería? Esta será una función bastante anémica (no hace mucho), más bien localizada (por lo tanto, no tiene un propósito general), y no se sostiene bien por sí sola (no puede entender para qué la necesita a menos que vea dónde está) usado). Otra opción es usar una macro, pero quiero hablar sobre las funciones en esta publicación.

Entonces, ¿deberías usar una función para algo como esto? ¿Cuáles son los pros y los contras?

EpsilonVector
fuente
2
El material de una sola línea necesita más que ser utilizado en múltiples lugares para garantizar pasar a una función separada.
1
Gran pregunta Ha habido tantas veces que me he preguntado lo mismo.
rdasxy

Respuestas:

31

Este es un gran uso de funciones.

Esta será una función bastante anémica (no hace mucho) ...

Eso es bueno. Las funciones solo deberían hacer una cosa.

más bien localizado ...

En un idioma OO, hazlo privado.

(por lo que no es de uso general)

Si maneja más de un caso, es de uso general. Además, generalizar no es el único uso de funciones. De hecho, están allí para (1) guardarlo escribiendo el mismo código más de una vez, pero también (2) para dividir el código en fragmentos más pequeños para que sea más legible. En este caso, está logrando ambos (1) y (2). Sin embargo, incluso si su función se llama desde un solo lugar, aún podría ayudar con (2).

y no se sostiene bien por sí mismo (no puedo entender para qué lo necesita a menos que vea dónde se usa).

Se le ocurre un buen nombre, y se mantiene bien por sí solo. "ValidateFileParameters" o algo así. Ahora se encuentra bien por sí solo.

Kramii reinstala a Monica
fuente
66
Bien puntos de bienes. Agregaría (3) evitar que busque un error duplicado en toda su base de código cuando puede solucionarlo en un solo lugar. La duplicación de código puede llevar a un infierno de mantenimiento bastante rápido.
deadalnix
2
@deadalnix: Buen punto. Mientras escribía me di cuenta de que mis puntos eran una simplificación excesiva. Escribir y depurar más fácilmente es sin duda un beneficio de dividir las cosas en funciones (como lo es la capacidad de probar unitariamente funciones separadas).
Kramii reinstala a Mónica el
11

Totalmente debería ser una función.

if (isBufferValid(buffer)) {
    // ...
}

Mucho más legible y más fácil de mantener (si alguna vez cambia la lógica de verificación, solo se cambia en un lugar).

Además, cosas como estas se alinean fácilmente, por lo que ni siquiera tiene que preocuparse por los gastos generales de las llamadas de función.

Déjame hacerte una mejor pregunta. ¿Cómo no hacer esto es una buena práctica?

Hacer lo correcto. :)

Ñame Marcovic
fuente
44
Si isBufferValid es simple, return buffer != null;entonces creo que estás perjudicando la legibilidad allí.
pdr
55
@pdr: En ese caso simple, solo perjudica la legibilidad si tienes una mentalidad de monstruo de control y realmente, realmente, REALMENTE quieres saber cómo el código verifica si el búfer es válido. Entonces es subjetivo en esos casos simples.
Spoike
44
@pdr No sé cómo perjudica la legibilidad por ningún estándar. Usted está eximiendo a otros desarrolladores de preocuparse por cómo está haciendo algo y de centrarse en lo que está haciendo. isBufferValides definitivamente más legible (en mi libro) que buffer != null, porque comunica el propósito más claramente. Y nuevamente, sin mencionar que también te salva de la duplicación aquí. ¿Qué más necesitas?
Yam Marcovic
5

En mi opinión, vale la pena mover los fragmentos de código a sus propias funciones siempre que esto haga que el código sea más legible , independientemente de si la función será muy corta o se usará solo una vez.

Por supuesto, hay límites dictados por el sentido común. No desea tener el WriteToConsole(text)método con el cuerpo simplemente Console.WriteLine(text), por ejemplo. Pero errar en el lado de la legibilidad es una buena práctica.

Konamiman
fuente
2

Generalmente es una buena idea usar funciones para eliminar la duplicación en el código.

Sin embargo , puede llevarse demasiado lejos. Esta es una decisión judicial.

Para tomar el ejemplo de su verificación de búfer nulo, probablemente diría que el siguiente código es lo suficientemente claro y no debe extraerse en una función separada, incluso si se usa el mismo patrón en algunos lugares.

if (buf==null) throw new BufferException("Null read buffer!");

Si incluye el mensaje de error como un parámetro para una función de verificación nula genérica, y también considera el código requerido para definir la función, entonces no es un ahorro neto de LOC reemplazar esto con:

checkForNullBuffer(buf, "Null read buffer!");

Además, tener que sumergirse en la función para ver qué está haciendo al depurar significa que la llamada a la función es menos "transparente" para el usuario y, por lo tanto, puede considerarse menos legible / mantenible.

mikera
fuente
Podría decirse que su ejemplo dice más sobre la falta de soporte de contrato en el idioma que la necesidad real de duplicación. Pero buenos puntos de todos modos.
deadalnix
1
De alguna manera te perdiste el punto como yo lo veo. No tener que pasar por alto o tener que considerar la lógica cada vez que depure es lo que estoy buscando. Si quiero saber cómo se hace, lo reviso una vez. Tener que hacerlo cada vez es simplemente una tontería.
Yam Marcovic
Si se repite el mensaje de error ("Null read buffer"), esa duplicación definitivamente debería eliminarse.
Kevin Cline
Bueno, es solo un ejemplo, pero presumiblemente tendrá un mensaje diferente en cada sitio de llamada de función, por ejemplo, "Null read buffer" y "Null write buffer", por ejemplo. A menos que quiera los mismos mensajes de registro / error para cada situación que no puede de-duplicarlo .......
mikera
-1 Preconditions.checkNotNull es una buena práctica, no una mala. Sin embargo, no es necesario incluir el mensaje de cadena. google-collections.googlecode.com/svn/trunk/javadoc/com/google/…
ripper234
2

Centralizar el código suele ser siempre una buena idea. Debemos reutilizar el código tanto como sea posible.

Sin embargo, es importante tener en cuenta cómo hacerlo. Por ejemplo, cuando tiene un código que compute_prime_number () o check_if_packet_is_bad () es bueno. Lo más probable es que el algoritmo de funcionalidad en sí evolucione y se beneficie.

Sin embargo, cualquier código que se repita como prosa no califica para ser centralizado de inmediato. Esto es malo. Puede ocultar líneas de código arbitrarias dentro de una función solo para ocultar un código, con el tiempo, cuando varias partes de la aplicación comienzan a usarse, todas deben seguir siendo compatibles con las necesidades de todos los llamados de la función.

Aquí hay algunas preguntas que debe hacer antes de preguntar

  1. ¿La función que está creando tiene su propio significado inherente o es solo un montón de líneas?

  2. ¿Qué otro contexto requerirá el uso de las mismas funciones? ¿Es probable que requiera generalizar un poco la API antes de usar esto?

  3. ¿Cuál será la expectativa de (diferentes partes de) las aplicaciones cuando arroje excepciones?

  4. ¿Cuáles son los escenarios para ver que las funciones van a evolucionar?

También debe verificar si ya existen cosas como esta. He visto a muchas personas que siempre tienden a redefinir sus macros MIN, MAX en lugar de buscar lo que ya existe.

Esencialmente, la pregunta es: "¿Es esta nueva función realmente digna de reutilización o es solo una copia y pega ?" Si es primero, es bueno ir.

Dipan Mehta
fuente
1
Bueno, si el código duplicado no tiene su propio significado, su código le dice que necesita ser refactorizado. Porque los lugares donde ocurre la duplicación probablemente tampoco tengan su propio significado.
deadalnix
1

Se debe evitar la duplicación de código. Cada vez que lo anticipe, debe evitar la duplicación de código. Si no lo anticipó, aplique la regla de 3: refactorizar antes de que el mismo fragmento de código se duplique 3 veces, anote la peculiaridad cuando se duplique 2 veces.

¿Qué es una duplicación de código?

  • Una rutina que se repite en varios lugares en la base de código. Este tourine debe ser más complejo que una simple llamada de función (de lo contrario, no ganará nada factorizando).
  • Una tarea muy común, incluso trivial (a menudo una prueba). Esto mejorará la encapsulación y la semántica en el código.

Considere el siguiente ejemplo:

if(user.getPrileges().contains("admin")) {
    // Do something
}

se convierte

if(user.isAdmin()) {
    // Do something
}

Mejoraste la encapsulación (ahora puedes cambiar las condiciones para ser un administrador de forma transparente) y la semántica del código. Si se descubre un error en la forma en que verifica que el usuario es un administrador, no necesita lanzar toda su base de código y hacer una solución en todas partes (con el riesgo de olvidar uno y obtener una falla de seguridad en su aplicación).

deadalnix
fuente
1
Por cierto, el ejemplo ilustra la Ley de Deméter.
Marzo
1

DRY se trata de simplificar la manipulación del código. Acaba de tocar un punto fino sobre este principio: no se trata de minimizar la cantidad de tokens en su código, sino de crear puntos únicos de modificación para un código semánticamente equivalente . Parece que sus cheques siempre tienen la misma semántica, por lo que deben ponerse en una función en caso de que necesite modificarlos.

l0b0
fuente
0

Si lo ve duplicado, debe encontrar una manera de centralizarlo.

Las funciones son una buena manera (tal vez no sea la mejor, pero eso depende del idioma) Incluso si la función es anémica, como usted dice, eso no significa que seguirá siendo así.

¿Qué pasa si tienes que buscar algo más también?

¿Va a encontrar todos los lugares donde tiene que agregar el cheque adicional o simplemente cambiar una función?

Thanos Papathanasiou
fuente
... por otro lado, ¿qué pasa si hay una probabilidad muy alta de que nunca le agregues algo? ¿Su sugerencia sigue siendo el mejor curso de acción en ese caso también?
EpsilonVector
1
@EpsilonVector mi regla de oro es que si tengo que cambiarlo una vez, también podría refactorizarlo. Entonces, en su caso, lo dejaría y si tuviera que cambiarlo, se convertiría en una función.
Thanos Papathanasiou
0

Casi siempre es bueno si se cumplen las siguientes condiciones:

  • mejora la legibilidad
  • usado dentro del alcance limitado

En un ámbito más amplio, debe sopesar cuidadosamente la duplicación frente a las compensaciones de dependencia. Ejemplos de técnicas de limitación de alcance: ocultarse en secciones o módulos privados sin exponerlos al público.

Mar
fuente