Pregunta

Despues de leer "¿Cuál es su/un buen límite para la complejidad ciclomática?", me doy cuenta de que muchos de mis colegas estaban bastante molestos con esta nueva control de calidad política sobre nuestro proyecto:no más 10 complejidad ciclomática por función.

Significado:no más de 10 declaraciones de bifurcación del flujo de trabajo de código 'if', 'else', 'try', 'catch'.Bien.Como expliqué en '¿Pruebas el método privado?', una política así tiene muchos efectos secundarios positivos.

Pero:Al comienzo de nuestro proyecto (200 personas, 7 años de duración), estábamos felizmente iniciando sesión (y no, no podemos delegar eso fácilmente en algún tipo de 'Programación Orientada a Aspectos' enfoque para registros).

myLogger.info("A String");
myLogger.fine("A more complicated String");
...

Y cuando se lanzaron las primeras versiones de nuestro sistema, experimentamos un gran problema de memoria no debido al registro (que en un momento estuvo desactivado), sino debido a la parámetros de registro (las cadenas), que siempre se calculan, luego se pasan a las funciones 'info()' o 'fine()', solo para descubrir que el nivel de registro estaba 'OFF' y que no se estaba realizando ningún registro.

Entonces el control de calidad regresó e instó a nuestros programadores a realizar registros condicionales.Siempre.

if(myLogger.isLoggable(Level.INFO) { myLogger.info("A String");
if(myLogger.isLoggable(Level.FINE) { myLogger.fine("A more complicated String");
...

Pero ahora, con ese límite de 10 niveles de complejidad ciclomática por función que 'no se puede mover', argumentan que los diversos registros que colocan en su función se sienten como una carga, porque cada "if(isLoggable())" es ¡Contado como +1 de complejidad ciclomática!

Entonces, si una función tiene 8 'si', 'si no', etc., en un algoritmo estrechamente acoplado que no se puede compartir fácilmente, y 3 acciones de registro críticas...incumplen el límite a pesar de que los registros condicionales pueden no ser en realidad parte de dicha complejidad de esa función...

¿Cómo abordaría esta situación?
He visto un par de evoluciones de codificación interesantes (debido a ese "conflicto") en mi proyecto, pero primero quiero conocer su opinión.


Gracias por todas las respuestas.
Debo insistir en que el problema no está relacionado con el "formato", sino con la "evaluación de argumentos" (evaluación que puede ser muy costosa, justo antes de llamar a un método que no hará nada)
Entonces, cuando escribí arriba "A String", en realidad quise decir aFunction(), con aFunction() devolviendo una String y siendo una llamada a un método complicado que recopila y calcula todo tipo de datos de registro para ser mostrados por el registrador...o no (de ahí el problema, y ​​el obligación utilizar el registro condicional, de ahí el problema real del aumento artificial de la 'complejidad ciclomática'...)

Ahora obtengo el 'variado punto de función avanzado por algunos de ustedes (gracias John).
Nota:una prueba rápida en java6 muestra que mi función varargs evalúa sus argumentos antes de ser llamado, por lo que no se puede aplicar para la llamada de función, sino para el 'objeto de recuperación de registros' (o 'envoltorio de función'), en el que toString() solo se llamará si es necesario.Entiendo.

Ya he publicado mi experiencia sobre este tema.
Lo dejaré ahí hasta el próximo martes para votar, luego seleccionaré una de tus respuestas.
De nuevo, gracias por todas las sugerencias :)

¿Fue útil?

Solución

En Python, pasa los valores formateados como parámetros a la función de registro.El formato de cadena solo se aplica si el registro está habilitado.Todavía existe la sobrecarga de una llamada a función, pero eso es minúsculo en comparación con el formato.

log.info ("a = %s, b = %s", a, b)

Puede hacer algo como esto para cualquier lenguaje con argumentos variados (C/C++, C#/Java, etc.).


En realidad, esto no está pensado para cuando los argumentos son difíciles de recuperar, sino para cuando formatearlos en cadenas es costoso.Por ejemplo, si su código ya tiene una lista de números, es posible que desee registrar esa lista para depurar.ejecutando mylist.toString() Tomará un tiempo sin ningún beneficio, ya que el resultado se desperdiciará.entonces pasas mylist como parámetro de la función de registro y dejar que maneje el formato de cadena.De esa manera, el formateo sólo se realizará si es necesario.


Dado que la pregunta del OP menciona específicamente Java, así es como se puede usar lo anterior:

Debo insistir en que el problema no está relacionado con el "formato", sino con la "evaluación de argumentos" (evaluación que puede ser muy costosa, justo antes de llamar a un método que no hará nada)

El truco consiste en tener objetos que no realicen cálculos costosos hasta que sea absolutamente necesario.Esto es fácil en lenguajes como Smalltalk o Python que admiten lambdas y cierres, pero aún es factible en Java con un poco de imaginación.

Digamos que tienes una función get_everything().Recuperará todos los objetos de su base de datos en una lista.Obviamente, no desea llamar a esto si el resultado se descartará.Entonces, en lugar de usar una llamada a esa función directamente, defines una clase interna llamada LazyGetEverything:

public class MainClass {
    private class LazyGetEverything { 
        @Override
        public String toString() { 
            return getEverything().toString(); 
        }
    }

    private Object getEverything() {
        /* returns what you want to .toString() in the inner class */
    }

    public void logEverything() {
        log.info(new LazyGetEverything());
    }
}

En este código, la llamada a getEverything() está envuelto para que no se ejecute hasta que sea necesario.La función de registro se ejecutará toString() en sus parámetros sólo si la depuración está habilitada.De esa manera, su código sufrirá sólo la sobrecarga de una llamada a función en lugar de la carga completa. getEverything() llamar.

Otros consejos

Con los marcos de registro actuales, la pregunta es discutible

Los marcos de registro actuales como slf4j o log4j 2 no requieren declaraciones de protección en la mayoría de los casos.Usan una declaración de registro parametrizada para que un evento pueda registrarse incondicionalmente, pero el formato del mensaje solo ocurre si el evento está habilitado.La construcción del mensaje la realiza el registrador según lo necesita, en lugar de hacerlo de forma preventiva la aplicación.

Si tiene que utilizar una biblioteca de registro antigua, puede seguir leyendo para obtener más información y una forma de actualizar la biblioteca antigua con mensajes parametrizados.

¿Las declaraciones de los guardias realmente añaden complejidad?

Considere excluir las declaraciones de los guardias madereros del cálculo de la complejidad ciclomática.

Se podría argumentar que, debido a su forma predecible, las comprobaciones de registro condicionales realmente no contribuyen a la complejidad del código.

Las métricas inflexibles pueden hacer que un programador que de otro modo sería bueno se vuelva malo.¡Ten cuidado!

Suponiendo que sus herramientas para calcular la complejidad no se puedan adaptar a ese grado, el siguiente enfoque puede ofrecer una solución alternativa.

La necesidad de un registro condicional

Supongo que tus declaraciones de guardia se introdujeron porque tenías un código como este:

private static final Logger log = Logger.getLogger(MyClass.class);

Connection connect(Widget w, Dongle d, Dongle alt) 
  throws ConnectionException
{
  log.debug("Attempting connection of dongle " + d + " to widget " + w);
  Connection c;
  try {
    c = w.connect(d);
  } catch(ConnectionException ex) {
    log.warn("Connection failed; attempting alternate dongle " + d, ex);
    c = w.connect(alt);
  }
  log.debug("Connection succeeded: " + c);
  return c;
}

En Java, cada una de las declaraciones de registro crea una nueva StringBuilder, e invoca el toString() método en cada objeto concatenado a la cadena.Estos toString() Los métodos, a su vez, probablemente creen StringBuilder instancias propias, e invocar el toString() métodos de sus miembros, y así sucesivamente, en un gráfico de objetos potencialmente grande.(Antes de Java 5, era aún más caro, ya que StringBuffer se utilizó y todas sus operaciones están sincronizadas).

Esto puede resultar relativamente costoso, especialmente si la declaración de registro se encuentra en alguna ruta de código muy ejecutada.Y, escrito como arriba, ese costoso formateo de mensajes ocurre incluso si el registrador descarta el resultado porque el nivel de registro es demasiado alto.

Esto lleva a la introducción de declaraciones de guardia de la forma:

  if (log.isDebugEnabled())
    log.debug("Attempting connection of dongle " + d + " to widget " + w);

Con esta guardia, la evaluación de argumentos. d y w y la concatenación de cadenas se realiza sólo cuando es necesario.

Una solución para un registro simple y eficiente

Sin embargo, si el registrador (o un contenedor que escriba en el paquete de registro elegido) toma un formateador y argumentos para el formateador, la construcción del mensaje se puede retrasar hasta que esté seguro de que se utilizará, eliminando al mismo tiempo las declaraciones de protección y sus Complejidad ciclomática.

public final class FormatLogger
{

  private final Logger log;

  public FormatLogger(Logger log)
  {
    this.log = log;
  }

  public void debug(String formatter, Object... args)
  {
    log(Level.DEBUG, formatter, args);
  }

  … &c. for info, warn; also add overloads to log an exception …

  public void log(Level level, String formatter, Object... args)
  {
    if (log.isEnabled(level)) {
      /* 
       * Only now is the message constructed, and each "arg"
       * evaluated by having its toString() method invoked.
       */
      log.log(level, String.format(formatter, args));
    }
  }

}

class MyClass 
{

  private static final FormatLogger log = 
     new FormatLogger(Logger.getLogger(MyClass.class));

  Connection connect(Widget w, Dongle d, Dongle alt) 
    throws ConnectionException
  {
    log.debug("Attempting connection of dongle %s to widget %s.", d, w);
    Connection c;
    try {
      c = w.connect(d);
    } catch(ConnectionException ex) {
      log.warn("Connection failed; attempting alternate dongle %s.", d);
      c = w.connect(alt);
    }
    log.debug("Connection succeeded: %s", c);
    return c;
  }

}

Ahora, nada de la cascada toString() Se producirán llamadas con sus asignaciones de buffer. ¡a menos que sean necesarios!Esto elimina efectivamente el impacto en el rendimiento que llevó a las declaraciones del guardia.Una pequeña penalización, en Java, sería el empaquetado automático de cualquier argumento de tipo primitivo que pase al registrador.

Podría decirse que el código que realiza el registro es incluso más limpio que nunca, ya que la concatenación desordenada de cadenas ha desaparecido.Puede ser aún más limpio si las cadenas de formato se externalizan (usando un ResourceBundle), que también podría ayudar en el mantenimiento o localización del software.

Otras mejoras

También tenga en cuenta que, en Java, un MessageFormat El objeto podría usarse en lugar de un "formato". String, que le brinda capacidades adicionales, como un formato de elección para manejar números cardinales de manera más ordenada.Otra alternativa sería implementar su propia capacidad de formato que invoque alguna interfaz que usted defina para "evaluación", en lugar de la básica. toString() método.

En los lenguajes que admiten expresiones lambda o bloques de código como parámetros, una solución sería darle precisamente eso al método de registro.Ese podría evaluar la configuración y solo si es necesario llamar/ejecutar el bloque de código/lambda proporcionado.Aunque aún no lo he probado.

Teóricamente Esto es posible.No me gustaría usarlo en producción debido a los problemas de rendimiento que espero con el uso intensivo de lamdas/bloques de código para el registro.

Pero como siempre:en caso de duda, pruébelo y mida el impacto en la carga y la memoria de la CPU.

¡Gracias por todas tus respuestas!Ustedes molan :)

Ahora bien, mis comentarios no son tan sencillos como los suyos:

Sí para un proyecto (como en 'un programa implementado y ejecutándose por sí solo en una única plataforma de producción'), supongo que puedes volverte técnico conmigo:

  • objetos dedicados 'Log Retriever', que se pueden pasar a un contenedor Logger, solo es necesario llamar a toString()
  • utilizado junto con un registro función variada (¡o una matriz de Objeto[] simple!)

y ahí lo tienes, como lo explican @John Millikin y @erickson.

Sin embargo, este problema nos obligó a pensar un poco en '¿Por qué exactamente estábamos iniciando sesión en primer lugar?'
Nuestro proyecto consta en realidad de 30 proyectos diferentes (de 5 a 10 personas cada uno) implementados en varias plataformas de producción, con necesidades de comunicación asincrónica y arquitectura de bus central.
El registro simple descrito en la pregunta estaba bien para cada proyecto. al principio (hace 5 años), pero desde entonces tenemos que dar un paso adelante.Introducir el KPI.

En lugar de pedirle a un registrador que registre algo, le pedimos a un objeto creado automáticamente (llamado KPI) que registre un evento.Es una llamada simple (myKPI.I_am_signaling_myself_to_you()) y no necesita ser condicional (lo que resuelve el problema del "aumento artificial de la complejidad ciclomática").

Ese objeto KPI sabe quién lo llama y, dado que se ejecuta desde el principio de la aplicación, puede recuperar muchos datos que estábamos calculando previamente en el momento cuando iniciamos sesión.
Además, ese objeto KPI se puede monitorear de forma independiente y calcular/publicar según demanda su información en un bus de publicación único e independiente.
De esa manera, cada cliente puede solicitar la información que realmente desea (como, "¿ha comenzado mi proceso y, en caso afirmativo, desde cuándo?"), en lugar de buscar el archivo de registro correcto y buscar una cadena críptica...

De hecho, la pregunta "¿por qué exactamente estábamos registrando en primer lugar?" Nos hizo darnos cuenta de que no estábamos registrando solo para el programador y su unidad o pruebas de integración, sino para una comunidad mucho más amplia, incluidos algunos de los propios clientes finales.Nuestro mecanismo de 'información' tenía que ser centralizado, asincrónico, 24 horas al día, 7 días a la semana.

Lo específico de ese mecanismo de KPI está fuera del alcance de esta pregunta.Baste decir que su calibración adecuada es, con diferencia, el problema no funcional más complicado al que nos enfrentamos.¡Todavía pone al sistema de rodillas de vez en cuando!Sin embargo, si está correctamente calibrado, puede salvar vidas.

Nuevamente, gracias por todas las sugerencias.Los consideraremos para algunas partes de nuestro sistema cuando el registro simple todavía esté vigente.
Pero el otro objetivo de esta pregunta era ilustrarles un problema específico en un contexto mucho más amplio y complicado.
Espero que les haya gustado.Podría hacer una pregunta sobre KPI (que, créanlo o no, ¡no está en ninguna pregunta sobre SOF hasta ahora!) a finales de la próxima semana.

Dejaré esta respuesta para votación hasta el próximo martes, luego seleccionaré una respuesta (obviamente no esta;))

Tal vez esto sea demasiado simple, pero ¿qué pasa con el uso del "método de extracción" para refactorizar la cláusula de protección?Su código de ejemplo de esto:

public void Example()
{
  if(myLogger.isLoggable(Level.INFO))
      myLogger.info("A String");
  if(myLogger.isLoggable(Level.FINE))
      myLogger.fine("A more complicated String");
  // +1 for each test and log message
}

Se convierte en esto:

public void Example()
{
   _LogInfo();
   _LogFine();
   // +0 for each test and log message
}

private void _LogInfo()
{
   if(!myLogger.isLoggable(Level.INFO))
      return;

   // Do your complex argument calculations/evaluations only when needed.
}

private void _LogFine(){ /* Ditto ... */ }

En C o C++ usaría el preprocesador en lugar de las declaraciones if para el registro condicional.

Pase el nivel de registro al registrador y déjelo decidir si escribe o no la declaración de registro:

//if(myLogger.isLoggable(Level.INFO) {myLogger.info("A String");
myLogger.info(Level.INFO,"A String");

ACTUALIZAR:Ah, veo que desea crear condicionalmente la cadena de registro sin una declaración condicional.Presumiblemente en tiempo de ejecución en lugar de tiempo de compilación.

Sólo diré que la forma en que hemos resuelto esto es poner el código de formato en la clase logger para que el formateo sólo se realice si se pasa el nivel.Muy similar a un sprintf incorporado.Por ejemplo:

myLogger.info(Level.INFO,"A String %d",some_number);   

Eso debería cumplir con sus criterios.

texto alternativo http://www.scala-lang.org/sites/default/files/newsflash_logo.png

escala tiene una anotación @elidable() que le permite eliminar métodos con un indicador del compilador.

Con la escala REPL:

C:>escala

Bienvenido a Scala versión 2.8.0.final (Java HotSpot(TM) VM de servidor de 64 bits, Java 1.6.0_16).Escriba expresiones para evaluarlas.Escriba: ayuda para obtener más información.

scala> import escala.annotation.Elidable Import Scala.annotation.Elidable

Scala> Import Scala.annotation.elidable._ import scaLA.Annotation.Elidable._

scala> @elidable(FINE) def logDebug(arg:String) = println(arg)

registroDepuración:(argumento:Cadena)Unidad

Scala> logDebug ("prueba")

escala>

Con elide-beloset

C:>scala -Xelide-por debajo de 0

Bienvenido a Scala versión 2.8.0.final (Java HotSpot(TM) VM de servidor de 64 bits, Java 1.6.0_16).Escriba expresiones para evaluarlas.Escriba: ayuda para obtener más información.

scala> import escala.annotation.Elidable Import Scala.annotation.Elidable

Scala> Import Scala.annotation.elidable._ import scaLA.Annotation.Elidable._

scala> @elidable(FINE) def logDebug(arg:String) = println(arg)

registroDepuración:(argumento:Cadena)Unidad

Scala> logDebug ("prueba")

pruebas

escala>

Ver también Definición de afirmación de Scala

El registro condicional es malo.Agrega desorden innecesario a su código.

Siempre debes enviar los objetos que tienes al registrador:

Logger logger = ...
logger.log(Level.DEBUG,"The foo is {0} and the bar is {1}",new Object[]{foo, bar});

y luego tener un java.util.logging.Formatter que usa MessageFormat para aplanar foo y bar en la cadena que se generará.Solo se llamará si el registrador y el controlador iniciarán sesión en ese nivel.

Para mayor placer, podría tener algún tipo de lenguaje de expresión para poder tener un control preciso sobre cómo formatear los objetos registrados (es posible que toString no siempre sea útil).

Por mucho que odio las macros en C/C++, en el trabajo tenemos #defines para la parte if, que si es falsa ignora (no evalúa) las siguientes expresiones, pero si es verdadera devuelve una secuencia a la que se pueden canalizar cosas usando el ' Operador <<'.Como esto:

LOGGER(LEVEL_INFO) << "A String";

Supongo que esto eliminaría la "complejidad" adicional que ve su herramienta y también eliminaría cualquier cálculo de la cadena o cualquier expresión que se registre si no se alcanzó el nivel.

Aquí hay una solución elegante que usa expresiones ternarias.

logger.info(logger.isInfoEnabled()?"La declaración de registro va aquí...":nulo);

Considere una función de utilidad de registro...

void debugUtil(String s, Object… args) {
   if (LOG.isDebugEnabled())
       LOG.debug(s, args);
   }
);

Luego haga la llamada con un "cierre" alrededor de la costosa evaluación que desea evitar.

debugUtil(“We got a %s”, new Object() {
       @Override String toString() { 
       // only evaluated if the debug statement is executed
           return expensiveCallToGetSomeValue().toString;
       }
    }
);
Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top