Comments 26
private static List<Integer> process(Map<String, Integer> options, List<String> inputs) {
List<Integer> res = new ArrayList<>();
int cur = -1;
for (String str : inputs) {
if (str.startsWith("-"))
if (options.containsKey(str)) {
if (cur == -1) cur = options.get(str);
}
else if (options.containsKey("+" + str)) {
if (cur == -1) cur = res.isEmpty() ? -1 :
res.remove(res.size() - 1);
if (cur != -1) res.add(cur + str.length());
}
}
return res;
}Ояэбу! Без 100 грамм не разобраться.
Вот тут ещё НПЕ можно выхватить:
public class ChildServiceImpl1Test {
@Test
public void name() {
final String key = "-azaza";
Map<String, Integer> map = new HashMap<String, Integer>();
map.put(key, null);
final List<String> inputs = Collections.singletonList(key);
process(map, inputs);
}
private static List<Integer> process(Map<String, Integer> options, List<String> inputs) {
List<Integer> res = new ArrayList<>();
int cur = -1;
for (String str : inputs) {
if (str.startsWith("-"))
if (options.containsKey(str)) {
if (cur == -1) cur = options.get(str);
}
else if (options.containsKey("+" + str)) {
if (cur == -1) cur = res.isEmpty() ? -1 :
res.remove(res.size() - 1);
if (cur != -1) res.add(cur + str.length());
}
}
return res;
}
}
Это натянуто. Ты в каждом методе, принимающем коллекцию, ожидаешь увидеть в ней null? Я вот нет. Иногда метод вправе ожидать каких-то гарантий относительно аргументов. Особенно приватный метод.
Иногда метод вправе ожидать каких-то гарантий относительно аргументов.
Согласен. Любопытно, зачем его сделали статическим? Неужто компилятору помочь хотели?
А зачем делать метод нестатическим, когда он может быть статическим?
Насколько я понимаю, в ООП статика считается плохим тоном. Здесь я вижу в статике только один смысл — облегчение работы компилятору.
Поэтому раздражает инспекция «Method may be static», и я её отключил.
Это просто дополнительная проверка, позволяющая заметить потенциальную проблему. Довольно часто она срабатывает тогда, когда метод действительно работает не так, как задумано. Я в таких случаях разбираюсь, почему так получилось, и почти всегда решаю порефакторить код.
Если завтра будет вызывать, завтра и поменяете. Давайте сразу добавим методу тридцать дополнительных параметров, ведь они завтра могут пригодиться. Получатель невиртуального метода — это просто один из его параметров.
Правильно вызывать через экземпляр бина.
Ну если вы придумали себе надуманные правила, то тогда да, правильно.
На выражении res.isEmpty() IDE говорит, что условие всегда истинно
Почему бы не говорить, что оно истинно при условии
(cur == -1)? Т.е. указать, что условие всегда истинно именно в этой ветке.Для человека, после такой подсказки, будет проще понять предмет недовольства анализатора.
Код как код, что-то преобразуется, что-то делается, но статическому анализатору он не понравился.
Если честно, то и мне этот код не особо нравится :)
Вообще, я иногда сталкиваюсь с подобными случаями, когда анализатор предлагает оптимизировать код в ущерб понятности и читаемости. Он, конечно, на 100% прав, но при последующей ревьюхе человекообразным существом у последнего могут возникнуть вопросы по поводу подобной оптимизации. К сожалению анализатор не предоставляет подробное теоретическое обоснование эквивалентности обоих вариантов, да и ревьюер в них особо вникать не будет — просто попросит: "оставь как раньше — так понятней".
То есть строчка 'C' действительно недостижима, а 'B' при этом может выполняться. Это возможно, только если условие res.isEmpty() действительно всегда истинно!
Не совсем так, на самом деле всегда истинно cur =! -1 || res.isEmpty(), т.е. как только список перестает быть пустым cur никогда не становится -1, а пока он -1 список пуст.
Хорошо, что статический анализатор находит ошибку в плохом коде, плохо что после исправления ошибки код остается плохим.
if (cur != -1) res.add(cur + str.length());
А как только cur != -1, большая часть веток перестаёт работать и cur не может стать обратно -1.
Следовательно !res.isEmpty => cur != -1
Что эквивалентно cur == -1 => res.isEmpty
Жаль, что аналогичный код на Котлине идея не смогла подсветить
Пожалуйста прячьте код под кат. Вы поломали всю rss ленту по ширине
Статический анализ IntelliJ IDEA против человеческого разума