Einfach, im Interesse der Allgemeinheit, Code-Analysatoren basieren, Java Fragen [geschlossen]

StackOverflow https://stackoverflow.com/questions/1582512

  •  21-09-2019
  •  | 
  •  

Frage

OK, nachdem einige Code Überprüfung mit PMD und FindBugs Codeanalysatoren, ich war in der Lage große Veränderungen auf dem überprüft Code zu tun. Allerdings gibt es einige Dinge, die ich nicht wissen, wie sie zu beheben. Ich werde iterieren sie unten und (für bessere Referenz) Ich werde jede Frage eine Nummer geben. Fühlen Sie sich frei zu einem / sie alle zu beantworten. Vielen Dank für Ihre Geduld.

1. Selbst schwere i einige der Regeln entfernt haben, sind die damit verbundenen Warnungen immer noch da, nachdem Sie den Code neu zu bewerten. Jede Idee, warum?

2. Bitte beachten Sie die Erklärungen: 

    private Combo comboAdress;

    private ProgressBar pBar;

und die Verweise auf Objekte, die von Getter und Setter: 

    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;
 }

Nun frage ich mich, warum die erste Erklärung geben Sie mir keine Warnung auf PMD, während die zweite mir die folgende Warnung gibt: 

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

Weitere Informationen zu dieser Warnung hier .

3. Hier ist eine weitere Warnung, auch von PMD gegeben: 

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

Weitere Informationen zu dieser Warnung hier .

Nun, ich stimme mit dem, aber was ist, wenn ich so etwas schreiben: 

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

Ich neige dazu, mit der Regel zustimmen, aber wenn die Leistung des Codes vorschlägt mehrere Austrittspunkte, was soll ich tun?

4. PMD gibt mir diese: 

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

, wenn ich erklären, so etwas wie: 

String start_page = null;

Ich werde diese Informationen zu befreien (Ebene der Warnung ist info), wenn ich die Zuordnung zu null entfernen, but..i bekam eine Fehlermeldung von IDE und sagte, dass der Variable nicht initialisiert, irgendwann später im Code werden kann. Also, ich bin ein bisschen fest damit. die Warnung Unterdrücken ist das Beste, was Sie tun können?

5. PMD Achtung: 

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

Dies ist der Fall einer Singletone Verwendung von GUI-Komponenten oder bei einer Methode, die komplexen Objekte zurückgibt. Zuweisung des Ergebnisses zu Null in der catch () Abschnitt ist es durch die Notwendigkeit gerechtfertigt, die Rückkehr eines unvollständigen / inkonsistent Objekt zu vermeiden. Ja, sollte NullObject verwendet werden, aber es gibt Fälle, in denen ich möchte nicht, dass zu tun. Soll ich unterdrücken, dass die Warnung dann?

6. FindBugs Warnung # 1: 

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

in der Methode 

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

der statischen Variablen 

private static MyClass instance = null;

Die Variable ermöglicht es mich zu testen, ob das Formular bereits erstellt und sichtbar ist oder nicht, und ich brauche die Neuschöpfung der Form zu zwingen, in einigen Fällen. Ich sehe keine andere Alternative hier. Keine Erkenntnisse? (MyClass Geräte Listener, daher der overrided handle () -Methode).

7. FindBugs Warnung # 2: 

Class MyClass2 has a circular dependency with other classes

Diese Warnung basiert auf einfachen Einfuhren von anderen Klassen angezeigt. Muss ich diese Einfuhren Refactoring auf diese Warnung weggehen? Oder das Problem beruht in MyClass2?

OK, genug gesagt für now..expect ein Update auf der Grundlage vertiefter Erkenntnisse und / oder Ihre Antworten. Danke.

War es hilfreich?

Lösung

Hier sind meine Antworten auf einige Ihrer Fragen:

Frage Nummer 2 :

Ich glaube, Sie sind nicht die Eigenschaften richtig kapitalisieren. Die Verfahren sollten genannt getPBar und setPBar werden. 

String pBar;

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

Die Java Beans-Spezifikation besagt, dass:

  

Für lesbare Eigenschaften wird es eine Getter-Methode sein, den Eigenschaftswert zu lesen. Für beschreibbaren Eigenschaften wird es eine Setter-Methode sein, der Wert der Eigenschaft zu ermöglichen, aktualisiert werden. [...] Konstruiert eine PropertyDescriptor für eine Eigenschaft, die die Standard-Java-Konvention folgt durch getFoo und setFoo Accessormethoden haben. Wenn also das Argument Name ist „fred“, geht sie davon aus, dass der Leser Methode „getFred“ und der Schriftsteller Methode wird als „setFred“. Beachten Sie, dass der Eigenschaftsname mit einem Kleinbuchstaben beginnen sollte, die in den Methodennamen aktiviert werden.

Frage Nummer 3 :

Ich stimme dem Vorschlag der Software Sie verwenden. Zur besseren Lesbarkeit wird nur ein Ausgangspunkt besser. Aus Gründen der Effizienz, mit der Rückkehr; ' könnte besser sein. Meine Vermutung ist, dass der Compiler ist intelligent genug, um immer die effiziente Alternative zu wählen, und ich wette, dass der Bytecode die in beiden Fällen gleich sein würde.

WEITERE INFORMATIONEN EMPIRICAL

ich einige Tests gemacht und herausgefunden, dass die Java-Compiler Ich verwende (javac 1.5.0_19 unter Mac OS X 10.4) ist die Optimierung nicht die Anwendung ich erwartet hatte.

Ich habe die folgende Klasse-Test: 

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();
}

Dann analysierte ich den Bytecode und festgestellt, dass für multReturn () gibt es mehrere 'iReturn Aussagen, während es nur eine für singleReturn () ist. Darüber hinaus auch der Bytecode von singleReturn () mehrere enthält goto auf die return-Anweisung.

testete ich beiden Methoden mit sehr einfachen Implementierungen von cond1, cond2 und cond3. Ich stellte sicher, dass die drei Bedingungen, bei denen ebenso beweisbar. Ich fand einen konsistenten Unterschied in der Zeit von 3% zu 6%, für multReturn () . In diesem Fall, da die Operationen sehr einfach sind, sind die Auswirkungen der Mehrfach Rückkehr deutlich spürbar.

Dann testete ich beiden Methoden unter Verwendung einer kompliziertere Implementierung von cond1, cond2 und cond3, um die Auswirkungen der unterschiedlichen Rückkehr weniger deutlich zu machen. Ich war von dem Ergebnis schockiert! Jetzt multReturn () ist konsequent langsamer als singleReturn () (zwischen 2% und 3%). Ich weiß nicht, was diesen Unterschied verursacht, weil der Rest des Codes soll gleich sein.

ich denke, diese unerwarteten Ergebnisse durch die JIT-Compiler der JVM verursacht werden.

Wie auch immer, ich stehe zu meiner anfänglichen Intuition. Den Compiler (oder JIT) diese Art von Dingen optimieren und diese frees der Entwickler auf das Schreiben von Code zu konzentrieren, die leicht lesbar und wartbar ist

Frage Nummer 6 :

Sie können eine Klassenmethode von Ihrer Instanz-Methode aufrufen und verlassen diese statische Methode die Klassenvariable ändern.

Dann Code Blick ähnlich der folgenden: 

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;
         }
    }
}

Dies würde bewirken, dass die Warnung Sie in 5 beschrieben, aber es hat einig Kompromiss sein, und in diesem Fall ist es nur ein Geruch, kein Fehler.

Frage Nummer 7 :

Das ist einfach ein Geruch eines möglichen Problems. Es ist nicht unbedingt schlecht oder falsch, und man kann nicht sicher sein, nur durch dieses Werkzeug verwenden.

Wenn Sie ein echtes Problem haben, wie Abhängigkeiten zwischen Konstrukteuren, Prüfung sollte es zeigen.

Ein andere, aber miteinander verbundene, Problem sind zirkuläre Abhängigkeiten zwischen Gläsern. Während Klassen mit zirkulären Abhängigkeiten kompiliert werden können, zirkuläre Abhängigkeiten zwischen Gläsern können nicht wegen der Art und Weise Klassenlade Arbeit in der JVM behandelt werden

Andere Tipps

  1. Ich habe keine Ahnung. Es scheint wahrscheinlich, dass das, was Sie getan haben, war es nicht das, was Sie zu tun versucht haben!

  2. Vielleicht erscheinen die Erklärungen in einer Serializable Klasse aber, dass die Art (z ComboProgress nicht selbst serializable). Wenn dieser UI-Code ist, dann scheint sehr wahrscheinlich. Ich würde die Klasse nur kommentieren, um anzuzeigen, dass es nicht serialisiert werden soll.

  3. Dies ist eine gültige Warnung. Sie können Ihren Code Refactoring so: 

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

  4. Das ist, warum ich nicht statische Analyse-Tools stehen kann. Eine null Zuordnung offensichtlich lässt Sie NullPointerExceptions später öffnen. Allerdings gibt es viele Orte, wo dies einfach unvermeidbar ist (z try catch finally mit Ressourcenbereinigung unter Verwendung eines Closeable zu tun)

  5. Dies scheint auch, wie eine gültige Warnung und die Nutzung von static Zugang wäre wahrscheinlich ein Code Geruch von den meisten Entwicklern in Betracht gezogen werden. Betrachten Sie Refactoring über Abhängigkeitsinjektion mit der Ressource-Tracker in die Klassen zu injizieren, wo Sie die statische im Moment verwenden.

  6. Wenn Ihre Klasse hat nicht verwendet Importe diese dann entfernt werden soll. Diese Macht machen die Warnungen verschwinden. Auf der anderen Seite, wenn die Importe erforderlich sind, können Sie eine echte zirkuläre Abhängigkeit haben, die so etwas wie dies: 

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

Dies ist normalerweise eine verwirrende Situation und Blätter Sie offen für ein Initialisierung Problem. Zum Beispiel können Sie versehentlich einige Codes in der Initialisierung von A hinzufügen, die seine B Instanz erfordert initialisiert werden. Wenn Sie einen ähnlichen Code in B hinzufügen, dann würde die zirkuläre Abhängigkeit bedeuten, dass der Code tatsächlich defekt war (das heißt konnte man weder eine A oder ein B konstruieren.

Wieder eine Darstellung, warum ich wirklich nicht wie statische Analyse-Tools - sie in der Regel liefern Sie dazu eine Reihe von Fehlalarmen. Der kreisabhängigen Code kann sehr gut arbeiten und sehr gut dokumentiert werden.

Für Punkt 3, wahrscheinlich die Mehrheit der Entwickler würde in diesen Tage sagen, dass die Single-Rückkehr Regel einfach flach falsch ist, und im Durchschnitt führt zu schlechten Code. Andere sehen, dass es ein abgeschriebene Regel mit historischen Credentials, dass einiger Code bricht es schwer zu lesen ist, und so folgt es nicht einfach falsch ist.

Sie scheinen mit dem ersten Lager zu einigen, aber es fehlt das Vertrauen das Werkzeug zu sagen, diese Regel zu deaktivieren.

Das ist daran zu erinnern ist, ist es eine einfache Regel, um Code in jedem Prüfinstrument, und einige Leute tun es wollen. So ist es ziemlich immer von ihnen umgesetzt werden.

Während wenige (wenn überhaupt) zur Durchsetzung der subjektive ‚Wache; Körper; Rückberechnung;‘ Muster, dass im Allgemeinen die am einfachsten zu lesen und einfachste Code erzeugt.

Wenn Sie also bei der Herstellung suchen guten Code , anstatt einfach nur die Vermeidung der schlimmsten Code , das eine Regel ist, dass Sie wahrscheinlich deaktivieren wollen.

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top