findbugs жалуется на автоматически сгенерированный код Eclipse

Вот метод hashCode(), который Eclipse любезно сгенерировал для меня:

@Override
public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + (int) (id ^ (id >>> 32));
    return result;
}

Когда я запускаю findbugs, он жалуется на последнюю строку:

Метод ...hashCode() сохраняет возвращаемый результат локально перед его немедленным возвратом [Самый страшный (2), Нормальная достоверность]

Кто здесь? Findbugs или Eclipse? Это изворотливо?

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

И все же это относится к категории Самый страшный!

Я что-то пропустил?

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


person chiastic-security    schedule 13.01.2015    source источник
comment
Я бы сказал, что ненужные строки ДЕЙСТВИТЕЛЬНО усложняют чтение и поддержку кода — FindBugs чувствует то же самое.   -  person colti    schedule 13.01.2015
comment
Этот пост похож на то, что вы спрашиваете, и содержит еще несколько ответов, которые здесь не перечислены. stackoverflow.com/questions/15078153 /   -  person austin wernli    schedule 14.01.2015
comment
Я не могу не заметить, что findbugs жалуется на код, который можно было бы упростить, просто возвращая выражение напрямую. Но он, по-видимому, думает, что умножение на переменную, которая, как известно, равна 1, — это просто прекрасно — в этом нет ничего сложного! Я чувствую, что пытаться использовать автоматизированный инструмент для обнаружения проблем со стилем в коде, сгенерированном другим автоматическим инструментом, не следует, кроме развлечения.   -  person ajb    schedule 14.01.2015
comment
Findbugs редко бывает прав в чем-либо; он просто выделяет вещи, которые не соответствуют настроенным критериям. По моему опыту, FB нуждается в стольких настройках, чтобы быть практичным, что становится непрактичным.   -  person E-Riz    schedule 14.01.2015
comment
@ajb Это не такой уж продвинутый инструмент. В основном это просто набор плагинов, каждый из которых соответствует одному шаблону. (конечно есть несколько более продвинутых)   -  person NiematojakTomasz    schedule 14.01.2015
comment
Предложил бы эту уловку: если вы можете использовать Project Lombok, вам не нужно будет использовать eclipse для создания этого метода, и FindBugs не сообщит об ошибке. В любом случае, меньше значит больше. projectlombok.org   -  person unigeek    schedule 14.01.2015


Ответы (4)


Кто здесь? Findbugs или Eclipse? Это изворотливо?

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

Иногда некоторые правила имеют практическую пользу. В некоторых случаях стиль Eclipse более удобочитаем (если у вас несколько полей). С другой стороны, как упоминалось в правиле @RomanC firebug, есть преимущество в подходящем контракте, заключающемся в том, что переменные не используются, если в этом нет необходимости.

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

Примечание. Инструменты проверки в стиле кода можно настраивать. Вы можете включить только те правила, которые сократят затраты на поддержку кода для вас, вашей команды или организации.

person NiematojakTomasz    schedule 13.01.2015
comment
Так как же обеспечить единый стиль кода для кода, автоматически сгенерированного каким-либо другим инструментом? - person ajb; 14.01.2015
comment
Это трудно. Вы можете использовать единый инструмент консультирования в стиле кода, такой как finbugs, PMD, checkstyle (с инструментом сборки, таким как интеграция maven), сонар, что-то встроенное в IDE и т. д. с набором правил, который вы решите использовать. Однако бесшовной интеграции между инструментами для создания кода и инструментами для создания кода не существует. Программистам обычно необходимо просматривать сгенерированный код, если вы хотите, чтобы код соответствовал правилам стиля кода. Проверка кода, встроенная в IDE, может быть очень полезной и позволяет сделать это с помощью нескольких щелчков мыши при настройке. - person NiematojakTomasz; 14.01.2015

Код должен быть упрощен, потому что он делает ненужные вещи, о которых вам сообщил firebug. Эта проверка действительна.

@Override
public int hashCode() {
    return 31 + (int) (id ^ (id >>> 32));
}
person Roman C    schedule 13.01.2015
comment
Однако ненужные вещи совершенно не мешают производительности. Максимум, что кто-либо мог бы действительно сказать по этому поводу, было бы основано на мнении. - person austin wernli; 13.01.2015
comment
Я пояснил в вопросе, что это конкретно проблема сохранения значения и его немедленного возврата, о котором я спрашиваю, потому что это то, на что жалуется findbugs. Я согласен, что все остальное можно было бы написать проще. - person chiastic-security; 14.01.2015
comment
Является ли самая страшная категория правильной категорией? Есть ли здесь потенциальная ошибка? Какая ошибка? Потому что, как и в случае с ОП, я этого не понимаю. - person MarnixKlooster ReinstateMonica; 14.01.2015
comment
Локальная переменная требует стека для хранения своих значений, вы можете обойтись без них, вы также можете подсчитать операции с использованием байт-кода и сравнить две реализации. нет узкого места в производительности, но есть небольшие накладные расходы. - person Roman C; 14.01.2015
comment
Компилятор с приличными возможностями оптимизации должен быть в состоянии понять, что в этом случае операция сохранения в локальную переменную в стеке может быть устранена. - person ajb; 14.01.2015
comment
@chiastic-security Сохраненное значение не используется в коде, а не в операторе возврата, на что жалуется firebug. - person Roman C; 14.01.2015
comment
@ajb Иногда пост-оптимизация сгенерированного кода может привести к созданию действительно нечитаемого кода. Даже если придерживаться правил форматирования кода и стиля. Люди до сих пор спорят об использовании табуляции или пробелов для отступа. - person NiematojakTomasz; 14.01.2015
comment
Все в байт-коде требует стека для хранения значения, будь то именованная переменная или анонимный промежуточный результат. Нет никакой разницы. - person Marko Topolnik; 14.01.2015
comment
Там какой-то нечитаемый код. Читаемый (и, следовательно, удобный для сопровождения) код гораздо важнее минимального кода. - person E-Riz; 14.01.2015
comment
@E-Riz Это прекрасно читается для моих глаз. Но это лишь доказывает, насколько это индивидуально и насколько бесплодно пытаться основывать на этом какие-либо объективные оценки. - person Marko Topolnik; 14.01.2015

Код Eclipse не так уж плох, но излишне неуклюж. В таких тривиальных случаях часто лучше написать его вручную. Ваш код по существу эквивалентен этому:

@Override public int hashCode() {
    return Long.hashCode(id);
}
person assylias    schedule 13.01.2015
comment
Это не отвечает на вопрос. - person martinez314; 14.01.2015
comment
@whiskeyspider Я думаю, что да - findbugs указывает на тот факт, что код неуклюж, и это ... - person assylias; 14.01.2015

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

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

public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + (int) (id ^ (id >>> 32));

    // I meant to do something more here with result, but forgot...

    return result;
}
person martinez314    schedule 13.01.2015