# Внутри Java Enterprise кода: проверка Flowable

Язык Java известен широким применением в бизнесе\. Бизнес\-процессами необходимо управлять, в чём может помочь платформа Flowable, которая, так сложилось, написана на Java, так ещё и с открытым исходным кодом\! А значит, мы можем изучить его с помощью PVS\-Studio и попробовать найти ошибки\.

![1195_flowable_article_ru/image1.png](https://import.viva64.com/docx/blog/1195_flowable_article_ru/image1.png)

## Очень страшные слова

Упомянутое в аннотации — это во многом это всё, что я могу сказать о проекте, не начав просто пересказывать написанное в Википедии или на официальном сайте\. Потому хорошей идеей будет обратиться к нему: [Flowable: Business Process Automation \| Low\-code \| Workflow Automation](https://www.flowable.com/)\.

Бизнес значит Enterprise\. А Java и Enterprise — известная связка\! [Следовательно,](https://projects.haykranen.nl/java/) большое количество строк кода совпадает по длине с эссе\. В связи с этим мне пришлось заняться своевольным форматированием для того, чтобы сделать код более читаемым\. Имейте в виду, что большая часть представленных фрагментов в оригинале, скорее всего, выглядит иначе\.

Чтобы убедиться, что мы работаем с [Java](https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition), предлагаю посмотреть несколько _коротких_ наименований классов:

* TimerChangeProcessDefinitionSuspensionStateJobHandler;
* IntermediateThrowCompensationEventActivityBehavior;
* AbstractExternalInvocationBpmnParseHandler;
* ProcessInstanceStartEventSubscriptionModificationBuilderImpl;
* UnfinishedHistoricActivityInstanceByProcessInstanceIdMatcher;
* ParallelMultiInstanceWithNoWaitStatesAsyncLeaveJobHandler;
* EventSubProcessVariableListenerlStartEventActivityBehavior\.

Но это не всё\. Рекордсменом по длине стал класс _UpdateChannelDefinitionTypeAndImplementationForAllChannelDefinitionsCmd_\. А с тестами побеждает это же наименование с дополнительным суффиксом _Test_\.

Посмотрев на Enterprise наименования, предлагаю заглянуть в кодовую базу\. Для этого, согласно традиции, воспользуемся PVS\-Studio и попробуем найти самое интересное\. Надеваем пиджаки и отправляемся в путь\.

## Мгновенное исполнение

Начинаем приключение с тестов\. Хотя тесты предназначены для поиска ошибок в логике программы, в них допустить ошибку так же просто, как и в обычном коде\. Тесты на тесты не пишут, а вот статический анализатор их может проверить\.

Причём про поиск ошибок в Unit\-тестах мы писали совсем недавно в [отдельной статье](https://pvs-studio.ru/ru/blog/posts/cpp/1179/)\! Рекомендую тестировщикам к ознакомлению\.

А здесь предлагаю взглянуть на следующий фрагмент:

```cpp
@Test
@CmmnDeployment
public void testExpressionReturnsFuture() {
  CountDownLatch latch = new CountDownLatch(2);
  TestBeanReturnsFuture testBean = new TestBeanReturnsFuture(latch, 
    cmmnEngineConfiguration.getAsyncTaskExecutor());
  CaseInstance caseInstance = cmmnRuntimeService.createCaseInstanceBuilder()
    .caseDefinitionKey("myCase")
    .transientVariable("bean", testBean)
    .start();

  assertCaseInstanceEnded(caseInstance);

  if (....) {
    HistoricCaseInstance historicCaseInstance = ....;

    // Every service task sleeps for 1s, 
    // but when we use futures it should take less then 2s.
    long durationInMillis = historicCaseInstance.getEndTime().getTime() – 
      historicCaseInstance.getEndTime().getTime();
    assertThat(durationInMillis).isLessThan(1500);
  }
}
```

Тест проверяет время исполнения сервиса\. К сожалению, значение _durationInMillis_ всегда равно нулю, поскольку вычитается _getEndTime\(\)_ из _getEndTime\(\)_\. Соответственно, сам тест, проверяющий, что это значение меньше 1500, всегда будет положительным, ведь 0 < 1500, что легко доказать после прочтения [Principia Mathematica](https://en.wikipedia.org/wiki/Principia_Mathematica)\.

Исправление крайне простое:

```cpp
long durationInMillis = historicCaseInstance.getEndTime().getTime() –
  historicCaseInstance.getStartTime().getTime();
```

Я перезапустил тест с этим исправлением, и он успешно прошёл\. А теперь посмотрим на сообщение анализатора PVS\-Studio, которое помогло найти проблему:

[V6001](https://pvs-studio.ru/ru/docs/warnings/v6001/) There are identical sub\-expressions historicCaseInstance\.getEndTime\(\)\.getTime\(\)' to the left and to the right of the '\-' operator\. ServiceTaskWithFuturesTest\.java 76

## BigDecimal

Далее на очереди легендарный в финтехе BigDecimal:

```cpp
public class ELExecutionContextBuilder {
  ....
  protected static void preProcessInputVariables(DecisionTable decisionTable,
    Map<String, Object> inputVariables) {
    ....
    // check if transformation is needed
    for (Map.Entry<String, Object> inputVariable : inputVariables.entrySet()) {
      String inputVariableName = inputVariable.getKey();
      try {
        Object inputVariableValue = inputVariable.getValue();
        if (inputVariableValue instanceof LocalDate) {
          Date transformedDate = ((LocalDate) inputVariableValue).toDate();
          inputVariables.put(inputVariableName, transformedDate);
        } else if (inputVariableValue instanceof java.time.LocalDate) {
          Date transformedDate = 
            Date.from(((java.time.LocalDate) inputVariableValue).atStartOfDay()
              .atZone(ZoneId.systemDefault())
              .toInstant());
          inputVariables.put(inputVariableName, transformedDate);
        } else if (....) {
          BigInteger transformedNumber = 
            new BigInteger(inputVariableValue.toString());
          inputVariables.put(inputVariableName, transformedNumber);
        } else if (inputVariableValue instanceof Double) {
          BigDecimal transformedNumber = 
            new BigDecimal((Double) inputVariableValue);
          inputVariables.put(inputVariableName, transformedNumber);
        } else if (inputVariableValue instanceof Float) {
          double doubleValue = 
            Double.parseDouble(inputVariableValue.toString());
          BigDecimal transformedNumber = new BigDecimal(doubleValue);   // <=
          inputVariables.put(inputVariableName, transformedNumber);
        }
      } catch (Exception ex) {
        throw new FlowableException(....);
      }
    }
  }
}
```

Рекомендую в код долго не всматриваться, иначе посмотрит в ответ\. Нас интересует работа с уже упомянутым _BigDecimal_\. Для выяснения деталей необходимо обратиться к примечанию в [JavaDoc](https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/math/BigDecimal.html#%3Cinit%3E(double)):

> The results of this constructor can be somewhat unpredictable\. One might assume that writing new BigDecimal\(0\.1\) in Java creates a BigDecimal which is exactly equal to 0\.1 \(an unscaled value of 1, with a scale of 1\), but it is actually equal to 0\.1000000000000000055511151231257827021181583404541015625\. This is because 0\.1 cannot be represented exactly as a double \(or, for that matter, as a binary fraction of any finite length\)\. Thus, the value that is being passed in to the constructor is not exactly equal to 0\.1, appearances notwithstanding\.

В этом же документе указана рекомендация к применению _BigDecimal\.valueOf\(double\)_ или _new BigDecimal\(String\)\. _При этом_ _в указанном фрагменте значение парсят в _String_, а затем вновь в _double_\. Изменится ли от этого что\-то? Предлагаю посмотреть интереса ради:

```cpp
void main() {
  double d = 0.1;
  BigDecimal bigDecimal;

  bigDecimal = new BigDecimal(d);
  println("new BigDecimal(double): \t" + bigDecimal.toPlainString());

  bigDecimal = new BigDecimal(String.valueOf(d));
  println("new BigDecimal(double -> string): \t" + 
    bigDecimal.toPlainString());

  bigDecimal = new BigDecimal(Double.parseDouble(String.valueOf(d)));
  println("new BigDecimal(double -> string -> double): \t" + 
    bigDecimal.toPlainString());

  bigDecimal = BigDecimal.valueOf(d);
  println("BigDecimal.valueOf(double): \t" + bigDecimal.toPlainString());
}
```

<details>
   <summary>void main\\\(\\\)? Это Java\\\-код?</summary>

Да\! Для подробностей предлагаю к изучению [JEP\-477](https://openjdk.org/jeps/477)\. Хотя на момент написания статьи этот функционал находится в preview feature, его использование в публикации помогает упростить такой простой фрагмент и не писать _public static void main\(String\[\] args\)_\. Великосимвольных строк нам хватит и в Enterprise названиях классов\.


</details>
Получаем следующий вывод:

```cpp
new BigDecimal(double): 
  0.1000000000000000055511151231257827021181583404541015625
new BigDecimal(double -> string): 0.1
new BigDecimal(double -> string -> double):
  0.1000000000000000055511151231257827021181583404541015625
BigDecimal.valueOf(double): 0.1
```

Что ж, от дополнительных преобразований _double_ в строку и обратно поведение не меняется\. В любом случае следует использовать либо _valueOf_, либо конструктор со _String_ аргументом, если мы хотим получить 0\.1 из вводимого 0\.1 значения \(исходя из названия метода, это обработка _input variables_\)\.

В результате получаем, что можно не только избавиться от потенциально неожиданного значения, но и устранить лишнюю операцию парсинга _String_ к _double_:

```cpp
new BigDecimal(inputVariableValue.toString());
```

Напоследок срабатывание анализатора PVS\-Studio:

[V6068](https://pvs-studio.ru/ru/docs/warnings/v6068/) Constructor call can result in imprecise representation of the initialized value\. ELExecutionContextBuilder\.java 137

## Пробел всегда в конце

```cpp
public class CronExpression implements Serializable, Cloneable {
  ....
  protected int findNextWhiteSpace(int i, String s) {
    for (; i < s.length() && (s.charAt(i) != ' ' || s.charAt(i) != '\t'); i++) {
    }

    return i;
  }
  ....
}
```

Сразу отмечу, что и в оригинале тело цикла пустое \(иначе бы там стояли \.\.\.\.\)\.

Уже встречавшаяся ошибка в других проектах — одно из выражений всегда истинно\. Речь о _s\.charAt\(i\) \!\= '  ' \|\| s\.charAt\(i\) \!\= '\\t'_\. 

Для того, чтобы получить ложный результат оператора ИЛИ, необходимо, чтобы символ был одновременно равен двум разным символам\. Что, естественно, невозможно\.

В результате получается, что цикл выполняется, пока условие _i < s\.length\(\)_ является _true_, и вывод метода — это всегда индекс конца строки, а не символа пробела\.

Предупреждение PVS\-Studio:

[V6007](https://pvs-studio.ru/ru/docs/warnings/v6007/) Expression 's\.charAt\(i\) \!\= '\\t'' is always true\. CronExpression\.java 963

## Бессмертные тесты

```cpp
@Test
void convertJsonToModelKeyWithFixedValue() {
  ChannelModel channelModel = readJson(....);
  assertThat(channelModel)
    .isInstanceOfSatisfying(KafkaOutboundChannelModel.class, model -> {
      model.getRecordKey().getFixedValue().equals("customer");
    });
}

@Test
void convertJsonToModelKeyWithDelegateExpression() {
  ChannelModel channelModel = readJson(....);
  assertThat(channelModel)
    .isInstanceOfSatisfying(KafkaOutboundChannelModel.class, model -> {
      model.getRecordKey().getDelegateExpression().equals("${test}");
    });
}

@Test
void convertJsonToModelKeyWithEventField() {
  ChannelModel channelModel = readJson(....);
  assertThat(channelModel)
    B.isInstanceOfSatisfying(KafkaOutboundChannelModel.class, model -> {
      model.getRecordKey().getEventField().equals("customer");
    });
}
```

Небольшой круг, и мы вновь встречаем тесты\. И вновь они не падают\. Сначала посмотрим на срабатывания анализатора PVS\-Studio, а затем попытаемся исправить:

* [V6010](https://pvs-studio.ru/ru/docs/warnings/v6010/) The return value of function 'equals' is required to be utilized\. KafkaOutboundChannelJsonConverterTest\.java 45
* [V6010](https://pvs-studio.ru/ru/docs/warnings/v6010/) The return value of function 'equals' is required to be utilized\. KafkaOutboundChannelJsonConverterTest\.java 53
* [V6010](https://pvs-studio.ru/ru/docs/warnings/v6010/) The return value of function 'equals' is required to be utilized\. KafkaOutboundChannelJsonConverterTest\.java 61

Вероятно, разработчик по некой причине ожидал, что вторым аргументом метода _isInstanceOfSatisfying_ является _Function<T, Boolean\>_\. Хотя я не знаю, почему могло быть такое ожидание, на деле же метод принимает _Consumer<T\>_\. Возможно, дело в простой невнимательности, и разработчику могло показаться, что метод _equals_ эквивалентен _AssertEquals_\.

Для того, чтобы тест упал, необходимо выбросить исключение\. Просто обернём вызовы _equals_ в _assertThat\(\.\.\.\.\)\.isEqualTo\(\.\.\.\.\)_\. Например:

```cpp
assertThat(model.getRecordKey().getEventField()).isEqualTo("customer");
```

## Сам себе родитель

```cpp
public class ProcessExecutionLogger {
  protected void internalPopulateExecutionTree(....) {
    if (parentMapping.containsKey(parentNode.getId())) {
      for (....) {
        ....

        childNode.setParentNode(childNode);                        // <=
        parentNode.getChildNodes().add(childNode);

        internalPopulateExecutionTree(childNode, parentMapping);
      }
    }
  }
}
```

Здесь нарушенная иерархия приводит к тому, что у объекта _childNode_ родителем является он сам\. На необходимую зависимость косвенно указывает строка, следующая сразу после – _parentNode\.getChildNodes\(\)\.add\(childNote\)_\. То есть родитель о ребёнке знает, но не наоборот\. Любопытно, существует ли риск при итерации по таким нодам выйти в _StackOverflowError_?

Сообщение анализатора PVS\-Studio о странности:

[V6100](https://pvs-studio.ru/ru/docs/warnings/v6100/) An object is used as an argument to its own method\. Consider checking the first actual argument of the 'setParentNode' method\. ProcessExecutionLogger\.java 107

## Единственная итерация

```cpp
protected String getTargetNameSpace(XMLStreamReader xmlStreamReader) {
  String targetNameSpace = null;
  try {
    while (xmlStreamReader.hasNext()) {
      try {
        xmlStreamReader.next();
      } catch (Exception e) {
        LOGGER.debug("Error reading XML document", e);
        throw new DmnXMLException("Error reading XML", e);
      }
      targetNameSpace = xmlStreamReader.getNamespaceURI();
      break;
    }
  } catch (XMLStreamException e) {
    LOGGER.error("Error processing DMN document", e);
    throw new DmnXMLException("Error processing DMN document", e);
  }

  return targetNameSpace;
}
```

Знакомимся с неожиданным _break_, который сводит количество итераций к единице вне зависимости от содержимого в _xmlStreamReader_\. Возможно, так и задумывалось, но тогда следовало использовать _if_\. Очень странная обработка ресурса\.

Небольшое обращение к git позволяет отыскать предыдущую версию метода \(он был переименован\):

```cpp
protected boolean isDMN12(XMLStreamReader xtr) {
    try {
        while (xtr.hasNext()) {
            try {
                xtr.next();
            } catch (Exception e) {
                LOGGER.debug("Error reading XML document", e);
                throw new DmnXMLException("Error reading XML", e);
            }
            return DMN_12_TARGET_NAMESPACE.equals(xtr.getNamespaceURI());
        }
        return false;
    } catch (XMLStreamException e) {
        LOGGER.error("Error processing DMN document", e);
        throw new DmnXMLException("Error processing DMN document", e);
    }
}
```

В этой версии вместо _break_ используется _return_, и вновь без условия\. Я думаю, что в этом методе просто решили использовать _while_, потому что вызывается метод _xtr\.hasNext\(\)_\. 

Вне зависимости от изначальной идеи метода, PVS\-Studio справедливо ругается, ведь из\-за такого кода возникает слишком большое количество вопросов:

[V6037](https://pvs-studio.ru/ru/docs/warnings/v6037/) An unconditional 'break' within a loop\. DmnXMLConverter\.java 187

## Непустая коллекция

```cpp
protected void addRecipient(MimeMessage message, 
  Collection<String> recipients, 
  Message.RecipientType recipientType) {
  if (recipients == null || recipients.isEmpty()) {
    return;
  }
  Collection<String> newRecipients = recipients;
  Collection<String> forceRecipients = defaultsConfiguration.forceTo();
  if (forceRecipients != null && !forceRecipients.isEmpty()) {
    newRecipients = forceRecipients;
  }
  if (!newRecipients.isEmpty()) {
    for (String t : newRecipients) {
      try {
        message.addRecipient(recipientType, createInternetAddress(t));
      } catch (MessagingException e) {
        throw new FlowableMailException(....);
      }
    }
  }
}
```

Бессмысленная проверка\. _newRecipients_ принимает значение либо _forceRecipients_, либо _recipients_\. Оба перед этим проверяются на _isEmpty_\.

Бессмысленности прибавляет и то, что цикл _for \(String t : newRecipients\)_ и так не стал бы итерировать по пустой коллекции\.

На такой излишний код указал анализатор PVS\-Studio:

Expression '\!newRecipients\.isEmpty\(\)' is always true\. JakartaMailFlowableMailClient\.java 194

## Поле без значения

```cpp
public class CallbackData {
  protected String callbackId;
  protected String callbackType;
  protected String instanceId;
  protected String oldState;
  protected String newState;
  protected Map<String, Object> additionalData = null;

  public CallbackData(String callbackId, String callbackType, 
      String instanceId, String oldState, 
      String newState, Map<String, Object> additionalData) {
    this.callbackId = callbackId;
    this.callbackType = callbackType;
    this.instanceId = instanceId;
    this.oldState = oldState;
    this.newState = newState;
  }

  ....

  public Map<String, Object> getAdditionalData() {
    return additionalData;
  }

  public void setAdditionalData(Map<String, Object> additionalData) {
    this.additionalData = additionalData;
  }
}
```

Срабатывание PVS\-Studio:

[V6022](https://pvs-studio.ru/ru/docs/warnings/v6022/) Parameter 'additionalData' is not used inside constructor body\. CallbackData\.java 29

Несколько теоретический случай\. Использование разработчиком указанного конструктора может привести к неприятным последствиям, ведь при передаче в него _additionalData_ это значение не будет записано\. Произойдёт неприятное рождение _NullPointerException_\. Например, при использовании метода _getAdditionalData\(\)_, если ожидается, что он не вернёт _null_\.

## Возможный null

```cpp
protected void endAllHistoricActivities(....) {
  ProcessEngineConfigurationImpl processEngineConfiguration = CommandContextUtil
      .getProcessEngineConfiguration();
  if (!processEngineConfiguration.getHistoryLevel()
        .isAtLeast(HistoryLevel.ACTIVITY)) {
    return;
  }

  List<HistoricActivityInstanceEntity> historicActivityInstances = ....;

  Clock clock = processEngineConfiguration.getClock();
  for (var historicActivityInstance : historicActivityInstances) {
    historicActivityInstance.markEnded(deleteReason, clock.getCurrentTime());

    // Fire event
    FlowableEventDispatcher eventDispatcher = null;
    if (processEngineConfiguration != null) {                        // <=
      eventDispatcher = processEngineConfiguration.getEventDispatcher();
    }
    if (eventDispatcher != null && eventDispatcher.isEnabled()) {
      seventDispatcher.dispatchEvent(....),
      processEngineConfiguration.getEngineCfgKey());                 // <=
    }
  }
}
```

_CommandContextUtil\.getProcessEngineConfiguration\(\)_ может вернуть _null_\. И с учётом, что проверка на _null_ в коде происходит, это ожидается\. Чтобы прибавить уверенности, посмотрим на этот метод:

```cpp
public static ProcessEngineConfigurationImpl getProcessEngineConfiguration() {
    return getProcessEngineConfiguration(getCommandContext());
}

public static ProcessEngineConfigurationImpl
  getProcessEngineConfiguration(CommandContext commandContext) {
    if (commandContext != null) {
        return ....
    }
    return null;
}
```

Может ли _commandContext_ быть _null_? Продолжаем проход по цепочке вызовов:

```cpp
public static CommandContext getCommandContext() {
    return Context.getCommandContext();
}

public static CommandContext getCommandContext() {
    Stack<CommandContext> stack = getStack(commandContextThreadLocal);
    if (stack.isEmpty()) {
        return null;
    }
    return stack.peek();
}
```

Если stack пуст, то возвращается null\. Пока предлагаю на этом остановиться, проверять возможность получения пустой коллекции может быть трудно\. 

В результате вызов _processEngineConfiguration\.getHistoryLevel\(\) _рискует обернуться _NullPointerException_\.

Предупреждение анализатора:

[V6060](https://pvs-studio.ru/ru/docs/warnings/v6060/) The 'processEngineConfiguration' reference was utilized before it was verified against null\. TerminateEndEventActivityBehavior\.java 166, TerminateEndEventActivityBehavior\.java 179

## ClassCastException

```cpp
public class IntegerDataObject extends ValuedDataObject {

  @Override
  public void setValue(Object value) {
    if (value instanceof String &&
      !StringUtils.isEmpty(((String) value).trim())) {
      this.value = Integer.valueOf(value.toString());
    } else if (value instanceof Number) {
      this.value = (Integer) value;                  // <=
    }
  }

  @Override
  public IntegerDataObject clone() {
    IntegerDataObject clone = new IntegerDataObject();
    clone.setValues(this);
    return clone;
  }
}
```

Странная проверка на _Number_ с последующим кастом в _Integer_ \(а также _Long_ и _Double_ в других классах\-наследниках _ValuedDataObject_\)\. Возможно, разработчик ожидал, что это возымеет эффект, или, подобно примитивным типам, произойдёт соответствующая конвертация\. Однако на самом деле исполнение следующей программы закончится _ClassCastException_:

```cpp
void main() {
    Number integer = 528;
    println((Float)integer);
}
```

Результат:

```cpp
Exception in thread "main" java.lang.ClassCastException: 
  class java.lang.Integer cannot be cast to class java.lang.Float 
  (java.lang.Integer and java.lang.Float 
    are in module java.base of loader 'bootstrap')
        at Exception.main(Exception.java:3)
```

Потому, думаю, необходимо пересмотреть происходящее в этих методах\.

Предупреждения PVS\-Studio:

* [V6042](https://pvs-studio.ru/ru/docs/warnings/v6042/) The expression is checked for compatibility with type 'java\.lang\.Number' but is cast to type 'java\.lang\.Integer'\. IntegerDataObject\.java 27 
* [V6042](https://pvs-studio.ru/ru/docs/warnings/v6042/) The expression is checked for compatibility with type 'java\.lang\.Number' but is cast to type 'java\.lang\.Long'\. LongDataObject\.java 27 
* [V6042](https://pvs-studio.ru/ru/docs/warnings/v6042/) The expression is checked for compatibility with type 'java\.lang\.Number' but is cast to type 'java\.lang\.Double'\. DoubleDataObject\.java 27

## Правим code style

```cpp
@Override
protected DmnElement convertXMLToElement(XMLStreamReader xtr, 
  ConversionHelper conversionHelper) throws Exception {
  DecisionService decisionService = new DecisionService();
  decisionService.setId(xtr.getAttributeValue(null, ATTRIBUTE_ID));
  decisionService.setName(xtr.getAttributeValue(null, ATTRIBUTE_NAME));
  decisionService.setLabel(xtr.getAttributeValue(null, ATTRIBUTE_LABEL));

  boolean readyWithDecisionService = false;
  try {
    while (!readyWithDecisionService && xtr.hasNext()) {
      xtr.next();
      if (xtr.isStartElement() &&
          ELEMENT_OUTPUT_DECISION.equalsIgnoreCase(xtr.getLocalName())) {
        DmnElementReference ref = new DmnElementReference();
        ref.setHref(xtr.getAttributeValue(null, ATTRIBUTE_HREF));
        decisionService.addOutputDecision(ref);
      } if (xtr.isStartElement() &&
            ELEMENT_ENCAPSULATED_DECISION
              .equalsIgnoreCase(xtr.getLocalName())) {
        DmnElementReference ref = new DmnElementReference();
        ref.setHref(xtr.getAttributeValue(null, ATTRIBUTE_HREF));
        decisionService.addEncapsulatedDecision(ref);
      } if (xtr.isStartElement() && 
            ELEMENT_INPUT_DATA.equalsIgnoreCase(xtr.getLocalName())) {
        DmnElementReference ref = new DmnElementReference();
        ref.setHref(xtr.getAttributeValue(null, ATTRIBUTE_HREF));
        decisionService.addInputData(ref);
      } else if (xtr.isEndElement() && 
          getXMLElementName().equalsIgnoreCase(xtr.getLocalName())) {
        readyWithDecisionService = true;
      }
    }
  } catch (Exception e) {
    LOGGER.warn("Error parsing output entry", e);
  }
  return decisionService;
}
```

Хотел я здесь найти проблему, где использование _if_ вместо _else if _привело бы к исполнению последнего _else_ блока, даже если один из блоков _if_ отработал \(подобно тому, что я находил в проекте [GeoGebra](https://pvs-studio.ru/ru/blog/posts/java/1151/)\)\. Однако здесь всё как раз оканчивается _else if_, а значит, ничего трагичного не случится\.

В результате получаем лишь неприятное форматирование кода\. Чтобы улучшить читаемость, необходимо вынести _if_ на следующие строки, либо добавить _else_\. Второй вариант может быть лучше, поскольку это позволит избавиться от исполнения нескольких бессмысленных операций процессора\.

Срабатывания PVS\-Studio:

* [V6086](https://pvs-studio.ru/ru/docs/warnings/v6086/) Suspicious code formatting\. 'else' keyword is probably missing\. DecisionServiceXMLConverter\.java 45 
* [V6086](https://pvs-studio.ru/ru/docs/warnings/v6086/) Suspicious code formatting\. 'else' keyword is probably missing\. DecisionServiceXMLConverter\.java 49

## Лишняя проверка

```cpp
protected void leaveFlowNode(FlowNode flowNode) {
  ....

  // Determine which sequence flows can be used for leaving
  List<SequenceFlow> outgoingSequenceFlows = new ArrayList<>();
  for (SequenceFlow sequenceFlow : flowNode.getOutgoingFlows()) {

    String skipExpressionString = sequenceFlow.getSkipExpression();
    if (!SkipExpressionUtil.isSkipExpressionEnabled(
         skipExpressionString, sequenceFlow.getId(), 
         execution, commandContext)
      ) {

      if (!evaluateConditions                                        // <=
      || (evaluateConditions &&                                      // <=
          ConditionUtil.hasTrueCondition(sequenceFlow, execution) &&
         (defaultSequenceFlowId == null ||
          !defaultSequenceFlowId.equals(sequenceFlow.getId())))) {
        outgoingSequenceFlows.add(sequenceFlow);
      }

    } else if (flowNode.getOutgoingFlows().size() == 1 ||
        SkipExpressionUtil.shouldSkipFlowElement(
          skipExpressionString, sequenceFlow.getId(), execution, commandContext)
        ) {

      ....
      outgoingSequenceFlows.add(sequenceFlow);
    }
  }
  ....
}
```

Я обнаружил большое количество подобных фрагментов кода\. Условие в _if_ и так крайне громоздкое, так ещё и излишнее:

[V6007](https://pvs-studio.ru/ru/docs/warnings/v6007/) Expression 'evaluateConditions' is always true\. TakeOutgoingSequenceFlowsOperation\.java 220

Вход во второе выражение в операторе _или_ происходит только в случае, если _evaluateConditions_ является _false_\. Соответственно, в проверке \(_evaluateConditions && \.\.\.\)_ выражение слева бессмысленно\.

## Заключение

На этом пока завершаем экскурс по потенциальным ошибкам в проекте\. Можно заметить, что значительная их часть произошла банально из\-за невнимательности\. По этой же причине они не были замечены и на code review\. Не стоит винить разработчиков, ведь размеры одного PR могут вызвать желание никогда его не открывать\.

А вот инструменты статического анализа проверяют код вне зависимости от его размера\. С помощью PVS\-Studio получилось отыскать такие проблемные места\. При этом это далеко не всё, что есть в проекте\. И не пришлось долго изучать кодовую базу, чтобы сориентироваться в структуре проекта, достаточно было лишь запустить анализатор\. А интеграция в процесс разработки так и вовсе позволит сразу находить ошибки в коде\.

При этом [попробовать PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/try-free/) можно совершенно бесплатно :\)