Простые, общие интересы, основанные на коде анализаторах, вопросы Java

Question

ОК, просмотрев код с PMD и FindBugs анализаторами кода, я смог внести отличные изменения в рассмотренный код. Однако есть некоторые вещи, которые я не знаю, как исправить. Я буду повторять их ниже, и (для лучшей справки) я дам каждому вопросу номер. Не стесняйтесь отвечать на все/все из них. Спасибо за ваше терпение.

1. Даже жестко я удалил некоторые из правил, соответствующие предупреждения все еще существуют после переоценки кода. Любая идея почему?

---

2. Пожалуйста, посмотрите на декларациях:

private Combo comboAdress; 

    private ProgressBar pBar; 

и ссылки на объекты по добытчиками и сеттеров:

private final Combo getComboAdress() { 
    return this.comboAdress; 
} 

private final void setComboAdress(final Combo comboAdress) { 
    this.comboAdress = comboAdress; 
} 

private final ProgressBar getpBar() { 
    return this.pBar; 
} 

private final void setpBar(final ProgressBar pBar) { 
    this.pBar = pBar; 
} 

Теперь я удивляюсь, почему первое заявление не дайте мне предупреждение о PMD, а второе дает мне следующее предупреждение:

Found non-transient, non-static member. Please mark as transient or provide accessors. 

Подробнее об этом предупреждении here.

---

3. Вот еще одно предупреждение, а также дается PMD:

A method should have only one exit point, and that should be the last statement in the method 

Более подробную информацию о том предупреждения here.

Теперь я с этим согласен, но что, если я пишу что-то вроде этого:

public void actionPerformedOnModifyComboLocations() { 
    if (getMainTree().isFocusControl()) { 
     return; 
    } 
    ....//do stuffs, based on the initial test 
} 

Я склонен согласиться с правилом, но если исполнение кода предложить несколько точек выхода, что я должен делать ?

---

4. PMD дает мне это:

Found 'DD'-anomaly for variable 'start_page' (lines '319'-'322'). 

, когда я объявляю что-то вроде:

String start_page = null; 

избавиться от этой информации (уровень предупреждения инфо), если я удалить присваивание null, но ... я получил ошибку от IDE, заявив, что переменная может быть неинициализирована, в какой-то момент позже в коде. Итак, я как бы застрял в этом. Преодоление предупреждения - это лучшее, что вы можете сделать?

---

5. ПМД Предупреждение:

Assigning an Object to null is a code smell. Consider refactoring. 

Это случай использования singletone компонентов GUI или случае метода, который возвращает сложные объекты. Присвоение результата null в разделе catch() оправдано необходимостью избежать возврата неполного/несогласованного объекта. Да, NullObject следует использовать, но есть случаи, когда я не хочу этого делать. Должен ли я подавить это предупреждение?

---

6.FindBugs предупреждение # 1:

Write to static field MyClass.instance from instance method MyClass.handleEvent(Event) 

в методе

@Override 
public void handleEvent(Event e) { 
    switch (e.type) { 
     case SWT.Dispose: { 
      if (e.widget == getComposite()) { 
       MyClass.instance = null; 
      } 
     break; 
     } 
    } 
} 

статической переменной

private static MyClass instance = null; 

переменная позволяет мне проверить, является ли форма уже создана и видимым или нет, и в некоторых случаях мне нужно принудительно воссоздать форму. Другого выбора здесь я не вижу. Какие-нибудь идеи? (MyClass реализует Listener, следовательно, метод override в методе handleEvent()).

---

7. FindBugs предупреждение № 2:

Class MyClass2 has a circular dependency with other classes 

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

ОК, достаточно сказал, что на данный момент .. ожидайте обновления, основываясь на дополнительных выводах и/или ваших ответах. Благодарю.

Answer 1

Вот мои ответы на некоторые вопросы:

---

..

Номер вопроса :

Я думаю, что вы не используете свойства собственности должным образом. Эти методы следует называть getPBar и setPBar.

String pBar; 

void setPBar(String str) {...} 
String getPBar() { return pBar}; 

В спецификации JavaBeans, утверждает, что:

Для получения читаемых свойств будет метод геттер, чтобы прочитать значение свойства. Для доступных для записи свойств будет установлен метод setter, позволяющий обновить значение свойства. [...] Создает PropertyDescriptor для свойства, следующего за стандартным соглашением Java, с помощью методов getFoo и setFoo. Таким образом, если имя аргумента является «fred», предполагается, что метод чтения - «getFred», а метод записи - «setFred». Обратите внимание, что имя свойства должно начинаться с символа нижнего регистра, который будет заглавный в именах методов.

---

Вопрос номер :

Я согласен с предложением программного обеспечения вы используете. Для удобства чтения только одна точка выхода лучше. Для эффективности используйте 'return;' может быть лучше. Я предполагаю, что компилятор достаточно умен, чтобы всегда выбирать эффективную альтернативу, и я готов поспорить, что байт-код будет одинаковым в обоих случаях.

ДАЛЕЕ эмпирическая ИНФОРМАЦИЯ

Я сделал несколько тестов и выяснил, что Java компилятор я использую (JAVAC 1.5.0_19 на Mac OS X 10.4) не применяя оптимизацию я ожидал.

Я использовал следующий класс для теста:

public abstract class Test{ 

    public int singleReturn(){ 
    int ret = 0; 
    if (cond1()) 
     ret = 1; 
    else if (cond2()) 
     ret = 2; 
    else if (cond3()) 
     ret = 3; 

    return ret; 
    } 

    public int multReturn(){ 
    if (cond1()) return 1; 
    else if (cond2()) return 2; 
    else if (cond3()) return 3; 
    else return 0; 
    } 

    protected abstract boolean cond1(); 
    protected abstract boolean cond2(); 
    protected abstract boolean cond3(); 
} 

Затем я проанализировал байткод и обнаружил, что для multReturn() есть несколько «ireturn» заявления, в то время как есть только один для singleReturn(). Более того, байт-код singleReturn() также включает несколько goto в оператор return.

Я проверил оба метода с очень простыми реализациями cond1, cond2 и cond3. Я удостоверился в том, что три условия, которые одинаково доказуемы. Я обнаружил последовательную разницу во времени от 3% до 6%, в пользу multReturn(). В этом случае, поскольку операции очень просты, влияние множественного возврата весьма заметно.

Затем я протестировал оба метода, используя более сложную реализацию cond1, cond2 и cond3, чтобы сделать влияние различного возврата менее очевидным.Я был потрясен результатом! Теперь multReturn() постоянно медленнее чем singleReturn() (между 2% и 3%). Я не знаю, что вызывает эту разницу, потому что остальная часть кода должна быть одинаковой.

Я думаю, что эти неожиданные результаты вызваны JIT-компилятором JVM.

В любом случае, я согласен с моей начальной интуицией: компилятор (или JIT) может оптимизировать такие вещи, и это освобождает разработчика, чтобы сосредоточиться на написании кода, который легко читается и поддерживается.

---

Вопрос номер :

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

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

public static void clearInstance() { 
    instance = null; 
} 

@Override 
public void handleEvent(Event e) { 
    switch (e.type) { 
     case SWT.Dispose: { 
      if (e.widget == getComposite()) { 
       MyClass.clearInstance(); 
      } 
     break; 
     } 
    } 
} 

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

---

Вопрос номер :

Это просто запах возможной проблемы. Это не обязательно плохо или неправильно, и вы не можете быть уверены, просто используя этот инструмент.

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

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

Answer 2

1. Я понятия не имею. Кажется вероятным, что все, что вы делали, это не то, что вы пытались сделать!

2. Возможно, объявления появляются в классе Serializable, но тип (например, ComboProgress не являются сериализуемыми). Если это код пользовательского интерфейса, то это кажется очень вероятным. Я бы просто прокомментировал класс, указав, что он не должен быть сериализован.

3. Это действительное предупреждение. Вы можете реорганизовать свой код таким образом:

   public void actionPerformedOnModifyComboLocations() { 
       if (!getMainTree().isFocusControl()) { 
        ....//do stuffs, based on the initial test 
       } 
   } 
   

4. Вот почему я не могу стоять инструменты статического анализа. Задание null, очевидно, оставляет вас открытым до NullPointerException. Тем не менее, есть много мест, где это просто неизбежно (например, с помощью try catch finally сделать очистку ресурсов с использованием Closeable)

5. Это также, кажется, как действительное предупреждение и использование вами static доступа, вероятно, будет рассматриваться код запах от большинства разработчиков. Рассмотрим рефакторинг с помощью инъекции зависимостей, чтобы ввести ресурс-трекер в классы, в которых вы используете статический на данный момент.

6. Если у вашего класса неиспользованных импорт, их следует удалить. Это может сделать предупреждения исчезнуть. С другой стороны, если требуется импорт, вы можете иметь подлинную циклическую зависимость, которая является чем-то вроде этого:

   class A { 
       private B b; 
   } 
   class B { 
       private A a; 
   } 
   

Это, как правило, запутанное положение дел и оставляет вас открыты к Инициализация проблема. Например, вы можете случайно добавить код в инициализацию A, для которого требуется, чтобы его экземпляр B был инициализирован.Если добавить аналогичный код в B, то циклическая зависимость означает, что ваш код был на самом деле сломана (т.е. вы не могли бы построить либо A или B.

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

Answer 3

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

Вы, кажется, согласны с первым лагерем, но не уверены в том, что инструмент должен отключить это правило.

Следует помнить, что это простое правило кодировать в любом инструменте проверки, и некоторые люди этого хотят. Таким образом, они почти всегда реализуются ими.

Принимая во внимание, что немногие (если таковые имеются) обеспечивают более субъективную охрану; тело; расчет возврата; который обычно создает самый простой и понятный код.

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