Я запускаю SonarQube, чтобы проверить свой код, и обнаружил случай, в котором я не понимаю сообщенную ошибку.
Мой код:
private static final int BASE_ID = 100_000_000;
private boolean isValidId(Id id) {
return id.asInteger().isPresent() && id.asInteger().get() >= BASE_ID;
}
Метод asInteger возвращает Optional<Integer>.
Ошибка, которую я получаю от sonarqube,
Call "Optional#isPresent()" before accessing the value.
в обратной линии.
Я понимаю, что код в порядке, так как вторая часть if не будет выполняться, если первая неверна. Я знаю, что это можно решить с помощью .filter(..).isPresent(), но мне больше нравится такой способ.
Есть идеи, почему это произошло?
Что, если второй вызов id.asInteger() вернет пустой Optional?
А как насчет return id.asInteger().orElse(-1) >= BASE_ID;?




Вы можете написать это как одно заявление, кстати:
return id.asInteger()
.map(x -> x >= BASE_ID)
.orElse(false)
но жалобы эхолота вызваны тем, что в данном случае это ложное срабатывание.
Да, я знаю, что могу решить эту проблему с помощью разных методов, таких как тот, который я написал в самом вопросе: .filter(..).isPresent(), но мне больше нравится другой способ, и я хочу понять, почему это может происходить. Я имею в виду, вызывается isPresent () ...
Но SonarQube не может знать, возвращает ли каждый вызов asInteger () другое значение. Итак, во второй раз, когда вы вызываете get (), это может быть пустой Необязательный
Это не ложное срабатывание - сонар совершенно прав, указывая на это как на потенциальную проблему.
Sonarqube не может гарантировать, что два вызова id.asInteger() вернут один и тот же объект, например поскольку многопоточность могла изменить значение id между двумя вызовами, поэтому правильно указывается, что наличие не было должным образом протестировано.
Измените код, чтобы сначала назначить его локальной переменной, чтобы гарантировать, что isPresent() и get() вызываются для одного и того же объекта:
private boolean isValidId(Id id) {
Optional<Integer> idAsInteger = id.asInteger();
return idAsInteger.isPresent() && idAsInteger.get() >= BASE_ID;
}
Многопоточность даже не требуется: два вызова одного и того же метода не гарантируют возврата одного и того же, и я не уверен, что Sonar может гарантировать, что asInteger () идемпотентен.
@Andreas отличная точка, 1+, но я бы полностью написал это как одно утверждение, хотя
@JBNizet Sonar никогда не может гарантировать какие-либо свойства метода в другом классе, так как он даже не может гарантировать, что реализация будет такой же во время выполнения. Единственными исключениями могут быть хорошо известные (JDK) методы с точно определенным контрактом или формальные контракты, выраженные в виде аннотаций.
При работе с Optionals вам следует по возможности избегать .isPresent и .get. Использование этих методов не безопаснее, чем использование нулей, и противоречит функциональному духу. Необязательные параметры предназначены для функционального программирования и для типизированной альтернативы нулевых проверок.
SonarQube имеет ограниченные возможности анализа. Обычно он не может исключить все виды ложных срабатываний. Этот случай не является проблемой, потому что не рекомендуется использовать Optionals таким образом.
Не обязательно. Например, как бы вы не использовали isPresent, когда вы хотите предпринять какое-то действие, когда значение отсутствует.
@tsolakp Начиная с Java 9 yourOptional.ifPresentOrElse(dummy, this::someAction);. Однако ваша точка зрения верна, это один из редкий (и, возможно, даже востребованных?) Случаев, когда можно защищать использование isPresent().
Да, я не собирался говорить «вы не должны использовать isPresent», но «в большинстве случаев было бы лучше избегать этого, если это возможно».
Чтобы избежать этой проблемы, я использую iterator (). Next (), он имеет те же функции, что и .get, но без проблемы с isPresent ()!
Я бы сказал, это просто потому, что эти инструменты проверки кода не всегда работают идеально, а некоторые ошибки являются просто ложными срабатываниями. ЕСЛИ вы перепишите его как
if (...), я уверен, что об ошибке больше не будет