Условное ведение журнала с минимальной цикломатической сложностью - PullRequest
62 голосов
/ 20 сентября 2008

Прочитав « Каков ваш / хороший предел для цикломатической сложности? », я понимаю, что многие мои коллеги были весьма недовольны этой новой политикой QA для нашего проекта: не более 10 цикломатическая сложность за функцию.

Значение: не более 10 'if', 'else', 'try', 'catch' и другой оператор ветвления потока кода. Правильно. Как я объяснил в « Вы тестируете закрытый метод? », такая политика имеет много хороших побочных эффектов.

Но: в начале нашего проекта (200 человек - 7 лет) мы счастливо регистрировались (и нет, мы не можем легко делегировать это какому-то «1013 * Аспектно-ориентированному программированию »). подход для бревен).

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

И когда появились первые версии нашей Системы, у нас возникла огромная проблема с памятью не из-за ведения журнала (которое в какой-то момент было отключено), а из-за параметров log (строки) , которые всегда вычисляются, затем передаются в функции 'info ()' или 'fine ()' только для того, чтобы обнаружить, что уровень ведения журнала был «ВЫКЛ» и что регистрация не выполнялась!

Итак, QA вернулась и призвала наших программистов вести условную регистрацию. Всегда.

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

Но теперь, с этим 10 цикломатическим уровнем сложности «не может быть перемещен» на предел функции, они утверждают, что различные журналы, которые они помещают в свою функцию, воспринимаются как бремя, потому что каждый «if (isLoggable ()» ) "считается +1 цикломатической сложностью!

Таким образом, если у функции есть 8 'if', 'else' и т. Д. В одном тесно связанном алгоритме, который трудно разделить, и 3 критических действия журнала ... они нарушают предел, даже если условные журналы могут не быть действительно частью упомянутой сложности этой функции ...

Как бы вы решили эту ситуацию?
Я видел пару интересных эволюций кодирования (из-за этого «конфликта») в моем проекте, но я просто хочу сначала высказать ваши мысли.


Спасибо за все ответы.
Я должен настаивать на том, что проблема связана не с «форматированием», а с «оценкой аргумента» (оценка может быть очень дорогостоящей, только перед вызовом метода, который ничего не будет делать)
Поэтому, когда написано выше «A String», я на самом деле имел в виду aFunction (), с aFunction (), возвращающей String, и являющейся вызовом сложного метода, собирающего и вычисляющего все виды данных журнала, которые должны отображаться регистратором ... или нет (отсюда проблема и обязательство использовать условное ведение журнала, отсюда и актуальная проблема искусственного увеличения «цикломатической сложности» ...)

Теперь я получаю точку ' variadic function', предложенную некоторыми из вас (спасибо, Джон).
Примечание: быстрый тест в java6 показывает, что моя функция varargs оценивает свои аргументы перед вызовом, поэтому она не может быть применена для вызова функции, но для «Объекта извлечения журнала» (или «обертки функции») , для которого toString () будет вызываться только при необходимости. Понял.

Я уже опубликовал свой опыт по этой теме.
Я оставлю его там до следующего вторника для голосования, а затем выберу один из ваших ответов.
Еще раз спасибо за все предложения :) 1050 *

Ответы [ 12 ]

53 голосов
/ 20 сентября 2008

С текущими каркасами ведения журналов вопрос является спорным

Текущие структуры ведения журналов, такие как slf4j или log4j 2, в большинстве случаев не требуют защитных операторов. Они используют параметризованный оператор журнала, чтобы событие можно было регистрировать безоговорочно, но форматирование сообщения происходит только в том случае, если событие включено. Построение сообщения выполняется по мере необходимости регистратором, а не в приоритетном порядке приложением.

Если вам нужно использовать античную библиотеку журналов, вы можете читать дальше, чтобы получить больше информации и способ дооснастить старую библиотеку параметризованными сообщениями.

Действительно ли охранные операторы добавляют сложности?

Подумайте об исключении инструкций охранников из расчета цикломатической сложности.

Можно утверждать, что из-за своей предсказуемой формы проверки условного журналирования действительно не способствуют сложности кода.

Негибкие метрики могут сделать хорошего программиста плохим. Будьте осторожны!

Предполагая, что ваши инструменты для расчета сложности не могут быть адаптированы к этой степени, следующий подход может предложить обходной путь.

Необходимость условного ведения журнала

Я предполагаю, что ваши защитные заявления были введены, потому что у вас был такой код:

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;
}

В Java каждый из операторов журнала создает новый StringBuilder и вызывает метод toString() для каждого объекта, присоединенного к строке. Эти методы toString(), в свою очередь, могут создавать собственные экземпляры StringBuilder и вызывать методы toString() их членов и т. Д. Для потенциально большого графа объектов. (До Java 5 он был еще дороже, поскольку использовался StringBuffer, и все его операции синхронизированы.)

Это может быть относительно дорого, особенно если оператор журнала находится в каком-то интенсивно выполняемом пути кода. И, как написано выше, это дорогостоящее форматирование сообщений происходит, даже если регистратор обязан отбросить результат из-за слишком высокого уровня журнала.

Это приводит к введению охранных заявлений вида:

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

С помощью этой защиты оценка аргументов d и w и конкатенация строк выполняются только при необходимости.

Решение для простой и эффективной регистрации

Однако, если регистратор (или оболочка, которую вы пишете вокруг выбранного вами пакета журналирования) принимает форматировщик и аргументы для форматера, создание сообщения может быть отложено до тех пор, пока не будет уверено, что оно будет использовано, при одновременном устранении защиты высказывания и их цикломатическая сложность.

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;
  }

}

Теперь, , ни один из каскадных toString() вызовов с их распределением буферов не произойдет , если они не нужны! Это эффективно устраняет снижение производительности, которое привело к защитным заявлениям. Одним небольшим штрафом в Java будет автоматическая упаковка любых аргументов примитивного типа, передаваемых в регистратор.

Код, ведущий запись в журнал, возможно, даже чище, чем когда-либо, так как неубранная конкатенация строк исчезла. Это может быть даже чище, если строки формата выводятся наружу (с использованием ResourceBundle), что также может помочь в обслуживании или локализации программного обеспечения.

Дальнейшие улучшения

Также обратите внимание, что в Java вместо "format" String может использоваться объект MessageFormat, который дает вам дополнительные возможности, такие как выбор формата для более аккуратной обработки кардинальных чисел. Другой альтернативой может быть реализация вашей собственной возможности форматирования, которая вызывает некоторый интерфейс, который вы определяете для «оценки», а не базовый метод toString().

30 голосов
/ 20 сентября 2008

В Python вы передаете отформатированные значения в качестве параметров функции регистрации. Форматирование строки применяется только в том случае, если включено ведение журнала. Затраты на вызов функции по-прежнему сохраняются, но это ничтожно мало по сравнению с форматированием.

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

Вы можете сделать что-то подобное для любого языка с переменными аргументами (C / C ++, C # / Java и т. Д.).


Это на самом деле не предназначено для случаев, когда аргументы трудно получить, но для форматирования их в строки стоит дорого. Например, если в вашем коде уже есть список чисел, вы можете записать этот список для отладки. Выполнение mylist.toString() займет некоторое время без пользы, так как результат будет отброшен. Таким образом, вы передаете mylist в качестве параметра функции регистрации и позволяете ей обрабатывать форматирование строки. Таким образом, форматирование будет выполняться только при необходимости.


Поскольку в вопросе OP конкретно упоминается Java, вот как можно использовать приведенное выше:

Я должен настаивать на том, что проблема связана не с «форматированием», а с «оценкой аргумента» (оценка может быть очень дорогостоящей, только перед вызовом метода, который ничего не будет делать)

Хитрость заключается в том, чтобы иметь объекты, которые не будут выполнять дорогостоящие вычисления до тех пор, пока в них не возникнет необходимость Это легко сделать в таких языках, как Smalltalk или Python, которые поддерживают лямбды и замыкания, но все же выполнимо в Java с небольшим воображением.

Скажем, у вас есть функция get_everything(). Он извлечет каждый объект из вашей базы данных в список. Вы не хотите вызывать это, если результат будет отброшен, очевидно. Поэтому вместо прямого вызова этой функции вы определяете внутренний класс с именем 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());
    }
}

В этом коде вызов getEverything() упакован так, что он фактически не будет выполнен, пока не понадобится. Функция ведения журнала будет выполнять toString() для своих параметров, только если включена отладка. Таким образом, ваш код будет страдать только от накладных расходов при вызове функции вместо полного getEverything() вызова.

6 голосов
/ 20 сентября 2008

В языках, поддерживающих лямбда-выражения или кодовые блоки в качестве параметров, одним из решений для этого было бы дать именно это методу регистрации. Он может оценить конфигурацию и только при необходимости вызвать / выполнить предоставленный блок лямбда / кода. Хотя пока не пробовал.

Теоретически это возможно. Я не хотел бы использовать его в производстве из-за проблем с производительностью, которые я ожидаю с таким интенсивным использованием блоков lamdas / code для регистрации.

Но как всегда: если есть сомнения, протестируйте его и измерьте влияние на загрузку процессора и память.

4 голосов
/ 28 сентября 2008

Может быть, это слишком просто, но как насчет использования рефакторинга "extract method" вокруг предложения guard? Ваш пример кода этого:

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
}

Становится так:

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 ... */ }
4 голосов
/ 20 сентября 2008

Спасибо за все ваши ответы! Вы, ребята, рок:)

Теперь мой отзыв не такой прямой, как у вас:

Да, для одного проекта (как в «одной программе, развернутой и работающей самостоятельно на одной производственной платформе»), я полагаю, вы можете использовать все технические данные для меня:

  • выделенные объекты 'Log Retriever', которые могут быть переданы оболочке Logger только с вызовом toString ()
  • используется вместе с логарифмируемой переменной функцией (или простым массивом Object []!)

и вот, у вас это есть, как объяснили @Джон Милликин и @Эриксон.

Однако эта проблема заставила нас задуматься о том, «Почему именно мы регистрировались в первую очередь?»
Наш проект на самом деле представляет собой 30 различных проектов (от 5 до 10 человек каждый), развернутых на различных производственных платформах с асинхронными коммуникационными потребностями и архитектурой центральной шины.
Простое ведение журнала, описанное в этом вопросе, было подходящим для каждого проекта в начале (5 лет назад), но с тех пор мы должны сделать шаг вперед. Введите KPI .

Вместо того, чтобы просить регистратора что-либо регистрировать, мы просим автоматически созданный объект (называемый KPI) зарегистрировать событие. Это простой вызов (myKPI.I_am_signaling_myself_to_you ()), и он не должен быть условным (что решает проблему «искусственного увеличения цикломатической сложности»).

Этот объект KPI знает, кто его вызывает, и, поскольку он запускается с самого начала приложения, он может получить много данных, которые мы ранее вычисляли на месте, когда мы регистрировали.
Кроме того, объект KPI можно контролировать независимо и вычислять / публиковать по требованию свою информацию на одной и отдельной шине публикации.
Таким образом, каждый клиент может запросить информацию, которую он на самом деле хочет (например, «начался ли мой процесс, и если да, с каких пор?»), Вместо поиска правильного файла журнала и поиска загадочной строки ...

Действительно, вопрос «Почему именно мы вошли в первую очередь?» заставили нас понять, что мы работали не только для программиста и его модульных или интеграционных тестов, но и для гораздо более широкого сообщества, включая самих конечных клиентов. Наш «отчетный» механизм должен был быть централизованным, асинхронным, 24/7.

Специфика этого механизма KPI является выходом за рамки этого вопроса. Достаточно сказать, что его правильная калибровка - это, безусловно, самая сложная нефункциональная проблема, с которой мы сталкиваемся. Это все еще время от времени ставит систему на колени! Однако при правильной калибровке это спасатель жизни.

Опять же, спасибо за все предложения. Мы рассмотрим их для некоторых частей нашей системы, когда простое ведение журнала все еще на месте.
Но другой смысл этого вопроса состоял в том, чтобы проиллюстрировать вам конкретную проблему в гораздо большем и более сложном контексте.
Надеюсь, вам понравилось. Я мог бы задать вопрос по KPI (которого, верьте или нет, пока нет в вопросе о SOF!) Позже на следующей неделе.

Я оставлю этот ответ на голосование до следующего вторника, затем выберу ответ (не этот, очевидно;))

3 голосов
/ 20 сентября 2008

Передайте уровень журнала в регистратор и дайте ему решить, писать или нет запись журнала:

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

ОБНОВЛЕНИЕ: Ах, я вижу, что вы хотите условно создать строку журнала без условного оператора. Предположительно во время выполнения, а не во время компиляции.

Я просто скажу, что способ, который мы решили, это поместить код форматирования в класс регистратора, чтобы форматирование происходило только при прохождении уровня. Очень похоже на встроенный спринтф. Например:

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

Это должно соответствовать вашим критериям.

3 голосов
/ 20 сентября 2008

В C или C ++ я бы использовал препроцессор вместо операторов if для условного ведения журнала.

2 голосов
/ 03 июля 2011

Условное ведение журнала - зло. Это добавляет ненужный беспорядок в ваш код.

Вы всегда должны присылать в журнал объекты, которые у вас есть:

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

, а затем есть java.util.logging.Formatter, который использует MessageFormat, чтобы сгладить foo и вставить строку для вывода. Он будет вызываться только в том случае, если регистратор и обработчик будут регистрироваться на этом уровне.

Для дополнительного удовольствия у вас может быть какой-то язык выражений, чтобы иметь возможность точно контролировать, как форматировать зарегистрированные объекты (toString не всегда может быть полезен).

2 голосов
/ 20 июля 2010

альтернативный текст http://www.scala -lang.org / sites / default / files / newsflash_logo.png

Scala имеет аннотацию @ elidable () , которая позволяет удалять методы с флагом компилятора.

С ответом scala:

C:> scala

Добро пожаловать в Scala версии 2.8.0.final (Java HotSpot ™), 64-разрядная серверная виртуальная машина, Java 1. 6.0_16). Введите выражения для их оценки. Введите: help для получения дополнительной информации.

scala> import scala.annotation.elidable импорт scala.annotation.elidable

scala> импорт scala.annotation.elidable._ импорт scala.annotation.elidable ._

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

logDebug: (arg: String) Единица

scala> logDebug ("тестирование")

Скала>

с элид-белосет

C:> scala -Xelide-ниже 0

Добро пожаловать в Scala версии 2.8.0.final (Java HotSpot ™), 64-разрядная серверная виртуальная машина, Java 1. 6.0_16). Введите выражения для их оценки. Введите: help для получения дополнительной информации.

scala> import scala.annotation.elidable импорт scala.annotation.elidable

scala> импорт scala.annotation.elidable._ импорт scala.annotation.elidable ._

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

logDebug: (arg: String) Единица

scala> logDebug ("тестирование")

тестирование

Скала>

См. Также Определение Scala Assert

1 голос
/ 22 ноября 2011

Рассмотрим функцию ведения журнала ...

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

Затем сделайте колл с «закрытием» вокруг дорогостоящей оценки, которую вы хотите избежать.

debugUtil(“We got a %s”, new Object() {
       @Override String toString() { 
       // only evaluated if the debug statement is executed
           return expensiveCallToGetSomeValue().toString;
       }
    }
);
...