Post on 28-May-2015
description
transcript
Gute Zeilen, schlechte ZeilenRegeln für wartbare Programme
Dirk Weil | GEDOPLAN
Dirk Weil• GEDOPLAN GmbH, Bielefeld• Java EE seit 1998• Konzeption und
Realisierung• Vorträge
dirk.weil@gedoplan.de
• Vorträge• Seminare• Veröffentlichungen
2 Gute Zeilen, schlechte Zeilen
GUTEZEILEN
dirk.weil@gedoplan.de3 Gute Zeilen, schlechte Zeilen
SCHLECHTE
ZEILEN
ZEILEN
Gibt es guten und schlechten Code?• Software ist (fast) nie fertig• Software wird (meist) im Team entwickelt• Teams ändern sich über die Zeit
dirk.weil@gedoplan.de
• Software muss verständlich sein
Gute Zeilen, schlechte Zeilen4
Gibt es guten und schlechten Code?• Entwicklerteams sind meist heterogen
– Berufserfahrung– Programmierstil– …
• Richtlinien helfen
dirk.weil@gedoplan.de
• Richtlinien helfen– bei der Einarbeitung in fremde Software– bei der (Weiter-) Entwicklung
Gute Zeilen, schlechte Zeilen5
Richtlinien
Low Level: Namen, Formatierung …
Grundlegendes Klassendesign
dirk.weil@gedoplan.de
Code-Komplexität
Anwendungsstruktur
6 Gute Zeilen, schlechte Zeilen
Statische Code-Analyse• Matching des Codes gegen Regelsätze• Einfache (Text-)Pattern … strukturelle Pattern
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen7
Checkstyle
Dokumentation• Javadoc für API
– Klassen, Interfaces,Methoden, Variablen
– public , protected
dirk.weil@gedoplan.de
– Kontrolle bspw. per CheckstyleJavadoc Comments
• Prüft per Default auch private� scope = protected
• Getter/Setter-Doku meist überflüssig� allowMissingPropertyJavadoc = true
Gute Zeilen, schlechte Zeilen8
Dokumentation• API-Dokumentation veröffentlichen
– Source-Jarserzeugen,z. B. mit Maven
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
<inherited>true</inherited>
<executions>
<execution>
<id>attach-sources</id>
dirk.weil@gedoplan.de
– ermöglicht Unter-stützung durchdie IDE
Gute Zeilen, schlechte Zeilen9
<id>attach-sources</id>
<phase>verify</phase>
<goals>
<goal>jar-no-fork</goal>
</goals>
</execution>
</executions>
</plugin>
Dokumentation• Erklärungsbedürftige Codesequenzen
/*
* Fahrstrassen innerhalb von Fahrstrassen expandieren, d. h. durch ihre Elemente
* ersetzen. Dies geschieht in einer Schleife solange, bis alle Expansionen
* erledigt sind oder kein Fortschritt mehr erzielt wird.
*/
int letzteAnzahlFahrstrassenFahrstrassen = 0;
int anzahlFahrstrassenFahrstrassen = 0;
dirk.weil@gedoplan.de
• Trivialdokumentation ist überflüssig
Gute Zeilen, schlechte Zeilen10
int anzahlFahrstrassenFahrstrassen = 0;
while (true)
{
for (Fahrstrasse fahrstrasse : this.fahrstrassen)
…
// Neue Weichenstellung protokollieren
this.logger.trace(this + ": setStellung(" + stellung + ")");
Namen• Klassen, Variablen, Methoden entsprechend ihrer
Aufgabe benennen– spart umfangreiche Dokumentation
• Well-Known Names nicht umdeuten!• Anschauungsbeispiel: "Nothalt-Funktion"
dirk.weil@gedoplan.de
• Anschauungsbeispiel: "Nothalt-Funktion"
– Anlagenstatus ist zu generell
– Methode setzt den Status nicht, sondern toggelt– setXyz ist well-known mit anderer Bedeutung
Gute Zeilen, schlechte Zeilen11
public void setAnlagenstatus()
{
Anlagenstatus.getInstance().changeAnlagenstatus();
Namen• Keine Präfixnotation für Typen, Sichtbarkeit etc.:
Instanzvariable Name beginnt mit m_
Variable vom Typ List<Integer> Name beginnt mit lI_
Interface Name beginnt mit I
… …
dirk.weil@gedoplan.de
– Präfixnamen tendieren zur Unlesbarkeit(was wäre wohl der Präfix für Map<String, Lok> ?)
– werden durch IDE-Unterstützung mehr als ersetzt
– this. ist aussagekräftiger (OO) als m_
– Unterscheidung Klasse vs. Interface zweitrangig
Gute Zeilen, schlechte Zeilen12
Namen• CS kann Namenskonventionen prüfen
(Module group Naming Conventions )
• IDE-Komfort nutzen!– Quick fix: Namensvorschläge (Variablen, Konstanten ..)– Save actions: Member mit this . qualifizieren
dirk.weil@gedoplan.de
– Save actions: Member mit this . qualifizieren
– …
Gute Zeilen, schlechte Zeilen13
Formatierung• Code sollte im Team einheitlich formatiert sein
– Einrückung (Tab/Blanks, wie viele?)– Platzierung von {
– Zeilenumbruch– …
dirk.weil@gedoplan.de
– …
• Vorteile– Code liest sich leichter– kleinere Change Sets im SCM
• Lässt sich mit IDE-Komfort leicht erreichen– Save action: Format code
Gute Zeilen, schlechte Zeilen14
equals , hashCode
• equals definieren � hashCode definieren• nicht nur equals (MyType )
• Codeanalyse:– CS: ,
dirk.weil@gedoplan.de
– CS: Equals and Hashcode , Covariant Equals
– FB: Class defines equals and uses Object.hashCode
Gute Zeilen, schlechte Zeilen15
equals , hashCode
• Jedes Geschäftsobjekt sollte equals und hashCode definieren
• IDEs bieten gute Unterstützung• Achtung bei equals in Basisklassen
public class BadEquals
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen16
public class BadEquals
{
public boolean equals(Object obj)
{
…
if (getClass() != obj.getClass())
// if (!(obj instanceof BadEquals)) // unsymmetrisch, wenn Subklasse überschreibt
// if (obj.getClass() != BadEquals.class) // funktioniert nicht für Subklassen
{
…
switch
• Regeln:– default nicht vergessen
– kein Fall Through
• CS: Missing Switch Default, Fall Through
dirk.weil@gedoplan.de17 Gute Zeilen, schlechte Zeilen
Protokollierung• keine Protokollausgabe auf stdout , stderr
• "Mal schnell 'ne Ausgabe"• Achtung: IDE-Templates!• CS: Regexp …mit passendem Pattern
dirk.weil@gedoplan.de18 Gute Zeilen, schlechte Zeilen
Exception-Verwendung• Werfen und Fangen von Throwable ,
Exception , RuntimeException i. A. fehlerhaft
• CS: Illegal Catch , Illegal Throws
dirk.weil@gedoplan.de19 Gute Zeilen, schlechte Zeilen
DRY• Keine Copy&Paste-Programmierung
• CS: Strict Duplicate Code(nicht wirklich empfehlenswert)
dirk.weil@gedoplan.de20 Gute Zeilen, schlechte Zeilen
Komplexität• Klassen / Methoden nicht zu lang• Anzahl Methodenparameter nicht zu groß• CS: Maximum Method Length , Maximum
Parameters , Maximum File Length , Cyclomatic Complexity
dirk.weil@gedoplan.de
Cyclomatic Complexity(Anwendung imTeam diskutieren!)
Gute Zeilen, schlechte Zeilen21
Einfach machen• Einfache Lösungen sind gute Lösungen• Vorsicht bei:
– Reflection• extrem schlecht lesbar• Refactoring problematisch
dirk.weil@gedoplan.de
• Refactoring problematisch
– hochgradig konfigurierbaren Klassen• schwer nutzbar (bspw. GridBagConstraints )
– übermäßigem Einsatz von Typparametern
Gute Zeilen, schlechte Zeilen22
Einfach machen• Anschauungsbeispiel: Entity Editor
– Generischer Editor für Geschäftsobjekte– Steuerung per Annotation @Editable– Remote funktionsfähig (� kein EntityManager)
dirk.weil@gedoplan.de
• Hochgradige Nutzung von Reflection• Umfangreiche Konfiguration von Services etc.
• Einsatzfall BDE-System– ca. 35 Entities
Gute Zeilen, schlechte Zeilen23
Klassen sparen lohnt nicht• Anschauungsbeispiel "Wachdienst":
– Gebäudekontrolle durch Prüfung aller Räume– Räume sind auf Etagen verteilt– Kontrollierte Räume
werden abgehakt
dirk.weil@gedoplan.de
werden abgehakt
Gute Zeilen, schlechte Zeilen24
Klassen sparen lohnt nicht• Anschauungsbeispiel "Wachdienst"
– 1. Ansatz: Keine Klasse für Etage
– Dialogaufbau unnötig kompliziert(~ Gruppenwechsel)
dirk.weil@gedoplan.de
(~ Gruppenwechsel)– Kein Platz für zukünftige Erweiterung
um Etagen-Daten
Gute Zeilen, schlechte Zeilen25
Klassen sparen lohnt nicht• Anschauungsbeispiel "Wachdienst"
– Besser: Zusätzliche Ebene für Etagen
– Klares KonzeptReal World � Klasse
dirk.weil@gedoplan.de
Real World � Klasse
Gute Zeilen, schlechte Zeilen26
Wohin mit der Logik?
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen27
Wohin mit der Logik?• Anschauungsbeispiel "Modellbahnsteuerung":
– Reservieren einer Fahrstraße= Stellen der betroffenen
Weichen und Signale– Fahrstrasse liegt als
dirk.weil@gedoplan.de
Geschäftsobjekt vor
– Anforderung als Webservice
28 Gute Zeilen, schlechte Zeilen
Wohin mit der Logik?• Anschauungsbeispiel "Modellbahnsteuerung":
– 1. Ansatz: Iteration über Fahrstraßenelemente im Webservice
– Nachteile:
@POST
@Path("/fahrstrasse/{bereich}/{name}/reserviert")
public Response setFahrstrassenreservierung(...)
{
Fahrstrasse fahrstrasse = …
dirk.weil@gedoplan.de
– Nachteile:• nicht wieder-
verwendbar, daim Webservice
• "Polymorphiefür Arme"
Gute Zeilen, schlechte Zeilen29
Fahrstrasse fahrstrasse = …
for (FahrstrassenElement fe
: fahrstrasse.getElemente())
{
Fahrwegelement fwe = fe.getFahrwegelement();
if (fwe instanceof Weiche)
{
Weiche w = (Weiche) fwe;
w.setStellung(…);
}
…
Wohin mit der Logik?• Anschauungsbeispiel "Modellbahnsteuerung":
– Besser:• Platzierung der Logik in Fahrstrasse• Abstrakte Methode statt expliziter Typabfrage
@Path("{bereich}/{name}/reserviert")
@POST
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen30
@POST
public Response setReserviert(…)
{
Fahrstrasse fahrstrasse = …
fahrstrasse.setReserviert(reserviert);
}
public void setReserviert(boolean reserviert)
{
for (FahrstrassenElement element : this.elemente)
element.setReserviert(reserviert);
public abstract void setReserviert(boolean reserviert);
Wohin mit der Logik?• Geschäftslogik in Geschäftslogik-Schicht
– CDI, EJB, JPA, …
• Präsentationslogik bspw. in JSF-Beans– CDI-Models, Managed Beans
• Boundary-Code in EJBs, Webservices etc.
dirk.weil@gedoplan.de
• Boundary-Code in EJBs, Webservices etc.
• Saubere Schichtung– erhöht Wiederverwendbarkeit– macht den Code übersichtlicher
Gute Zeilen, schlechte Zeilen31
In Objekten denken• Anschauungsbeispiel: 1:n-Relation (JPA)
@Entity
public class Publisher
{
@Id @GeneratedValue Integer id;
@OneToMany(mappedBy = "publisher") List<Book> books;
@Entity
public class Book
{
@Id @GeneratedValue Integer id;
@ManyToOne Publisher publisher;
dirk.weil@gedoplan.de
– Query "Suche Books eines Publishers"(Warum so kompliziert?):
Gute Zeilen, schlechte Zeilen32
@OneToMany(mappedBy = "publisher") List<Book> books;
…
Publisher publisher = …;
… em.createQuery("select b from Book b where b.publisher.id=:publisherId", Book.class)
.setParameter("publisherId", publisher.getId())
.getResultList();
Packages• Zwei Anti-Beispiele
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen33
Anekdoten
String name = new String();
name = "Hugo";
if (val != null && ("" + val.getClass().getName()).equals("java.lang.String"))
Set<String> texte = …;
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen34
Set<String> texte = …;
Set<Object> labels = new TreeSet<>();
labels.addAll(texte);
int adr = …;
String adrAsString = new Integer(adr).toString();
Anekdoten
public void changeRichtung()
{
if (isRueckwaerts())
setRueckwaerts(false);
else
setRueckwaerts(true);
}public class Anlagenstatus
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen35
public class Anlagenstatus
{
private static Anlagenstatus anlagenstatus = null;
private Anlagenstatus() { }
public static Anlagenstatus getInstance()
{
if (anlagenstatus == null)
anlagenstatus = new Anlagenstatus();
return anlagenstatus;
}
Anekdoten@POST
@Path("artikel/{artNr}/menge")
public Response setSpeed(@PathParam("artNr") String adrNr,
@FormParam("menge") String menge)
{
try
{
int mengeInt = Integer.parseInt(menge);
…
}
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen36
}
catch (NumberFormatException e)
{
Auto car1 = ...;
Auto car2 = ...;
if (car1.getId().getValue().equals(car2.getId().getValue()))
{
...
Anekdotenif (this.kommPunkt.isTurnText())
{
g2.setFont(system.getKommPunktFont());
g2.translate(kommPunktBreite / 2 + 2, 0 * kommPunktBreite / 2 - 1);
g2.rotate(Math.toRadians(this.kommPunkt.getDisplayWinkel() - 90),
this.kommPunkt.getDisplayX(), this.kommPunkt.getDisplayY());
g2.setColor(system.getColor(system.PROPERTY_KOMMPUNKT_TEXT_COLOR));
g2.drawString(this.kommPunkt.getId().getValue(), this.kommPunkt.getDisplayX() - 3,
this.kommPunkt.getDisplayY() + 2);
g2.setTransform(this.father.baseTransform);
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen37
g2.setTransform(this.father.baseTransform);
}
else
{
g2.setFont(system.getKommPunktFont());
g2.translate(kommPunktBreite / 2 + 2, 0 * kommPunktBreite / 2 - 1);
g2.rotate(Math.toRadians(this.kommPunkt.getDisplayWinkel() - 90),
this.kommPunkt.getDisplayX(), this.kommPunkt.getDisplayY());
g2.setColor(system.getColor(system.PROPERTY_KOMMPUNKT_TEXT_COLOR));
g2.drawString(this.kommPunkt.getId().getValue(), this.kommPunkt.getDisplayX() - 24,
this.kommPunkt.getDisplayY() + 2);
g2.setTransform(this.father.baseTransform);
}
dirk.weil@gedoplan.deGute Zeilen, schlechte Zeilen