Protokoll 2012-12-18 Review Armagan Kilic

Teilnehmer

• Küster
• Kilic
• Rathfelder
• Hauck

Demonstration Issue-Importer

• Quelle: http://sdqbuild.kit.edu/jira
• Benennung Issues -- woher kommen die Namen?

Decision-Modell

• Extension: dectrace-->decision

Plugin-Namen

• Top-Level Name für das "gesamte" Werkzeug
• de.fzi... vs. edu.kit...

Grundsätzliche Anmerkungen

• Generierten Code einchecken
• Exception-Handling: Welche Exception werden geworfen / gefangen?
  • Behandlung nicht in Eclipse-Console
  • Logging-Framework (log4j); nicht System.out.println()
  • Programmatische Fehler vs. Fehler aus der Benutzung
  • Dokumentieren, welche Exceptions geschluckt werden.
• Sehr viele String-Vergleiche --> Konstanten herausziehen (public static final String ...)
• Konstanten groß schreiben
• Graph von Decision-Modell trennen (nur noch aus Basis von String-Vergleichen)
  • Spezialisieren von GraphNodes

GraphFilter.java

• Hohe zyklomatische Komplexität

PCMDependencyGenerator

• Variablen-Namen mit vollem Namen
• Zähler wie xCounter um Kommentar erweitern
• Initialisierung im Konstruktor
• Konstante Zahlen (8, -20) als Konstanten herausziehen
• createDependentPCMNode: x umbenennen
  • Vergleich in Methode auslagern
  • Benennung: addXYZ
• generatedAllPCMDependencies
  • kapseln: Parameter einführen
  • Rekursion mit Parametern und expliziter Terminierung
• Z. 98: Was ist mit einem geschachtelten (nested) ResourceContainer (evtl. null?) --> Methode erwartet ein Argument
• Polymorphe Aufrufe (überladen) statt instanceof
• e als lokale Variable
• Z. 192: Welche Auswirkungen hat eine Änderung an den ui-classes?
• Z. 216: Was passiert, wenn Elemente gleich heißen? --> Besser den Identifier verwenden.
  • Nicht anhand von Labels prüfen, welche Elemente schon eingefügt sind
  • Entity statt Named Element (weil enthält dann eine ID)
• Klassenkommentar "generation of PCM entities" ist falsch

TraceLinkGenerator

• Entity statt NamedElement

RESTConnector

• Interface bauen, das ohne Variante der Kommunikation die Issues laden kann --> Trennung von Kommunikationstechnologie
• fetchIssue --> Exception (weiterwerfen? fangen?)
• authenticate --> Rückgabetyp statt Seiteneffekt

IssueImporter

• Benutzer und URL nicht in den COde, sondern als Parameter von main()

Kommentare Micha Hauck

• Warum wird generierter Code zu de.fzi.detrace.metamodel und de.fzi.issue.metamodel nicht eingecheckt?

RestConnector

• REST-unabhängige Logik (z.B. fetchBasicIssues) in ein Interface rausziehen. Der Verwender von RestConnector muss ja nicht wissen, dass die Kommunikation über REST läuft.

• fetchBasicIssues: searchJql wirft Exception, die nicht gefangen wird. Was soll damit passieren? Javadoc schreibt nichts darüber
  • System.out.println wo landet das? Gibt es eine Konsole? Besser logging-Framework verwenden
  • Comment "// A Basic Issue in JIRA has only key and link, so we get all of them and put in a loop." versteh ich nicht

• fetchIssue: getIssue wirft Exception, die nicht gefangen wird. Was soll damit passieren? Javadoc schreibt nichts darüber

• authenticate
  • JavaDoc "attempts to authentice": detaillieren. was passiert, wenn attempt fehlschlägt?
  • Exception in Z. 87 wird verschluckt
  • wo genau wird authentizifiert?

PCMDependencyGenerator

• Exceptions werden verschluckt

• private Variable g -> graph, n -> node, e -> elem.
  • Kommentare fehlen. Was ist xCounter?

• generateAllPCMDependencies():
  • statt instanceof besser polymorphe Aufrufe verwenden (Builder-Pattern)

L 98: Kann Methode ((ResourceContainer) e).getResourceEnvironment_ResourceContainer() null zurückgeben? L172/173: Warum xCounter = xCounter + 8; traceLinkCounter = -20; Kommentieren. Evtl. Konstanten verwenden

• graph attributes: string names in konstanten definieren, nur 1x definieren

• getNotGeneratedPCMNodes()
  • L 192: if (!next.getAttribute("ui.class").equals("decision") && !next.getAttribute("ui.class").equals("issue")) {

Warum ist sicher, dass alle anderen ui.class-Benennungen funktionieren?

• createDependentPCMNode()
  • L 216 n = g.addNode(x.toString()); besser n = g.addNode(x.getId());
  • L238 Exception wird geschluckt

TraceLinkGenerator

• L257 private void createRelatedPCMNodes(Entity x, Decision d)
  • warum Entity, nicht NamendEntity
  • x.getId() statt x.toString()
• L221 private void createRelatedDecisionNodes(Decision d)
  • Decision dr.toString(): ist toString() eineindeutig? (= ID)? wird vermutlich in L231/232 vorausgesetzt