Gute zeilen, schlechte zeilen - Regeln für wartbare Software

Gute Zeilen, schlechte Zeilen

Regeln für wartbare Programme

Dirk Weil, GEDOPLAN GmbH

Dirk Weil

GEDOPLAN GmbH, Bielefeld

Java EE seit 1998

Konzeption und

Realisierung

Vorträge

Seminare

Veröffentlichungen

Gute Zeilen, schlechte Zeilen 2

GUTE

SCHLECHTE

ZEILEN

ZEILEN


Gibt es guten und schlechten Code?

Software muss funktional korrekt sein

Software muss effizient bearbeitbar sein

Ist (fast) nie fertig

Wird (meist) im Team entwickelt

Teams ändern sich über die Zeit

unterschiedliche Berufserfahrung, Programmierstile, …

Software muss verständlich sein


Richtlinien

… helfen

bei der Einarbeitung in fremde Software

bei der (Weiter-) Entwicklung

Low Level: Namen, Formatierung …

Grundlegendes Klassendesign

Code-Komplexität

Anwendungsstruktur


Statische Code-Analyse

Matching des Codes gegen Regelsätze

Einfache (Text-)Pattern … strukturelle Pattern

Checkstyle


Dokumentation

Javadoc für API

Klassen, Interfaces,

Methoden, Variablen

public, protected

Kontrolle bspw. per Checkstyle

Javadoc Comments

Prüft per Default auch private

scope = protected

Getter/Setter-Doku meist überflüssig

allowMissingPropertyJavadoc = true


Dokumentation

API-Dokumentation

veröffentlichen

Source-Jars erzeugen,

z. B. mit Maven

ermöglicht Unter-

stützung durch

die IDE

<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-source-plugin</artifactId> <inherited>true</inherited> <executions> <execution> <id>attach-sources</id> <phase>verify</phase> <goals> <goal>jar-no-fork</goal> </goals> </execution> </executions> </plugin>


Dokumentation

Erklärungsbedürftige Codesequenzen

Trivialdokumentation ist überflüssig

/* * 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; 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"

Anlagenstatus ist zu generell

Methode setzt den Status nicht, sondern toggelt

setXyz ist well-known mit anderer Bedeutung

public void setAnlagenstatus() { Anlagenstatus.getInstance().changeAnlagenstatus();


Namen

Keine Präfixnotation für Typen, Sichtbarkeit etc.:

Präfixnamen tendieren zur Unlesbarkeit

this. ist aussagekräftiger (OO) als m_

Unterscheidung Klasse vs. Interface zweitrangig

werden durch IDE-Unterstützung mehr als ersetzt

Instanzvariable Name beginnt mit m_

Variable vom Typ List<Integer> Name beginnt mit lI_

Interface Name beginnt mit I

… …


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

…


Formatierung

Code sollte im Team einheitlich formatiert sein

Einrückung (Tab/Blanks, wie viele?)

Platzierung von {

…

Vorteile

Code liest sich leichter

kleinere Change Sets im SCM

Lässt sich mit IDE-Komfort leicht erreichen

Save action: Format code

Überprüfung durch Code Checker eher zweitrangig


equals, hashCode

Jedes Geschäftsobjekt sollte equals und hashCode definieren

IDEs bieten gute Unterstützung

Achtung bei equals in Basisklassen

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 { …


equals, hashCode

equals definieren hashCode definieren

nicht nur equals(MyType)

Codeanalyse:

CS: Equals and Hashcode, Covariant Equals

FB: Class defines equals and uses Object.hashCode


switch

Regeln:

default nicht vergessen

kein Fall Through

CS: Missing Switch Default, Fall Through


Protokollierung

keine Protokollausgabe auf stdout, stderr

"Mal schnell 'ne Ausgabe"

Achtung: IDE-Templates!

CS: Regexp… mit passendem Pattern


Exception-Verwendung

Werfen und Fangen von Throwable, Exception,

RuntimeException i. A. fehlerhaft

CS: Illegal Catch, Illegal Throws


DRY

Keine Copy&Paste-Programmierung

CS: Strict Duplicate Code

(nicht wirklich empfehlenswert)

PMD: Copy Paste Detection


Komplexität

Klassen / Methoden nicht zu lang

Anzahl Methodenparameter nicht zu groß

CS: Maximum Method Length, Maximum Parameters,

Maximum File Length,

Cyclomatic Complexity

(Anwendung im

Team diskutieren!)


Einfach machen

Einfache Lösungen sind gute Lösungen

Vorsicht bei:

Reflection

extrem schlecht lesbar

Refactoring problematisch

hochgradig konfigurierbaren Klassen

schwer nutzbar (bspw. GridBagConstraints)

übermäßigem Einsatz von Typparametern


Einfach machen

Anschauungsbeispiel: Entity Editor

Generischer Editor für Geschäftsobjekte

Steuerung per Annotation @Editable

Remote funktionsfähig ( kein EntityManager)

Hochgradige Nutzung von Reflection

Umfangreiche Konfiguration von Services etc.

Einsatzfall BDE-System

ca. 35 Entities


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



Anschauungsbeispiel "Wachdienst"

1. Ansatz: Keine Klasse für Etage

Dialogaufbau unnötig kompliziert

(~ Gruppenwechsel)

Kein Platz für zukünftige Erweiterung

um Etagen-Daten



Anschauungsbeispiel "Wachdienst"

Besser: Zusätzliche Ebene für Etagen

Klares Konzept

Real World Klasse


Wohin mit der Logik?



Anschauungsbeispiel "Modellbahnsteuerung":

Reservieren einer Fahrstraße

= Stellen der betroffenen

Weichen und Signale

Fahrstrasse liegt als

Geschäftsobjekt vor

Anforderung als Webservice




1. Ansatz: Iteration über Fahrstraßenelemente im Webservice

Nachteile:

nicht wieder-

verwendbar, da

im Webservice

"Polymorphie

für Arme"

(instanceof

~ goto der OO)

@POST @Path("/fahrstrasse/{bereich}/{name}/reserviert") public Response setFahrstrassenreservierung(...) { Fahrstrasse fahrstrasse = … for (FahrstrassenElement fe : fahrstrasse.getElemente()) { Fahrwegelement fwe = fe.getFahrwegelement(); if (fwe instanceof Weiche) { Weiche w = (Weiche) fwe; w.setStellung(…); } …




Besser: Platzierung der Logik in Fahrstrasse,

abstrakte Methode statt expliziter Typabfrage

@Path("{bereich}/{name}/reserviert") @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);



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.

Saubere Schichtung

erhöht Wiederverwendbarkeit

macht den Code übersichtlicher


In Objekten denken

Anschauungsbeispiel: 1:n-Relation (JPA)

Query "Suche Books eines Publishers"

(Warum so kompliziert?):

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

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


Anekdoten

String name = new String(); name = "Hugo";

if (val != null && ("" + val.getClass().getName()).equals("java.lang.String"))

Set<String> texte = …; Set<Object> labels = new TreeSet<>(); labels.addAll(texte);

int adr = …; String adrAsString = new Integer(adr).toString();

Offensichtlich komplett falsche Vorstellung

von Objekten und Referenzen darauf

instanceof

Sind die Labels nicht Texte? Warum dann Set<Object>?

Wenn unterschiedliche Typen erlaubt: Set<?>

Integer.toString(adr)


Anekdoten

public void changeRichtung() { if (isRueckwaerts()) setRueckwaerts(false); else setRueckwaerts(true); }

public class Anlagenstatus { private static Anlagenstatus anlagenstatus = null; private Anlagenstatus() { } public static Anlagenstatus getInstance() { if (anlagenstatus == null) anlagenstatus = new Anlagenstatus(); return anlagenstatus; }

setRueckwaerts(!isRueckwaerts())

public static final Anlagenstatus

= new Anlagenstatus()

Noch besser: enum-Singleton


Anekdoten

@POST @Path("artikel/{artNr}/menge") public Response setSpeed(@PathParam("artNr") String adrNr, @FormParam("menge") String menge) { try { int mengeInt = Integer.parseInt(menge); … } catch (NumberFormatException e) {

Auto car1 = ...; Auto car2 = ...; if (car1.getId().getValue().equals(car2.getId().getValue())) { ...

int menge

if (car1.equals(car2))


Anekdoten

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


Unit Tests

Test der funktionalen Korrektheit

auf Unit-Ebene

einzelne Klasse

isoliert von ihrer Umgebung, ggf. mittels Mock-Objekten

als Integrationstests

während der Entwicklung

als nachträglich Absicherung

oder als Vorgehensweise (Test Driven Development)

als Qualitätssicherungsmaßnahme

automatisiert

regelmäßig


Unit Tests

nicht aufwändiger als Main-Programm zum „Ausprobieren“

public class WaehrungServiceUnitTest

{

private static WaehrungService WAEHRUNG_SERVICE = new WaehrungService();

@Test

public void testUmrechnenUSD()

{

double actual = WAEHRUNG_SERVICE.umrechnen(100.0, "USD");

assertThat("Euro-Betrag", actual, is(83.41));

} public class WaehrungServiceMain

{

private static WaehrungService WAEHRUNG_SERVICE = new WaehrungService();

public static void main(String [] args)

{

BigDecimal actual = WAEHRUNG_SERVICE.umrechnen(100.0, "USD");

System.out.println("100 USD = " + actual + " EUR");

}


Unit Tests

Testwissen wird explizit gemacht

als Einstiegsdokumentation / Tutorial geeignet

automatisch, unbedient ausführbar

Continuous Integration (Hudson, Jenkins, …)

regelmäßige Ausführung aller Tests effizient möglich

dauerhafte Qualitätssicherung

unverzichtbar für Refactoring und Weiterentwicklung

Absicherung gegen „Verschlimmbesserung“


Continuous Integration

Manuelle Ausführung reicht nicht

belastet den Entwicklungsprozess

keine (einheitliche) Veröffentlichung der Ergebnisse

keine (einheitliche) Eskalation bei Fehlern

Bei mir läuft's! Oh, sorry – das habe

ich noch nicht

eingecheckt.

An dem Teil habe ich

nichts gemacht!



Voraussetzung: Projekt enthält ausführbare Tests

Junit, TestNG, …

Build selbst ist auch ein Test!



Anforderungen an eine Build- und Test-Umgebung

Regelmäßige, automatische Ausführung

zeitgesteuert ("Daily Build")

durch Check-In getriggert

…

Kompletter Build

Ausführung aller Tests

Qualitätsprüfung (Style, Coverage, …)

Reporting

Benachrichtigung



Entw.-

Umgebung

Plan

SCM

Test

Archiv

Feedback

Build

CI-Umgebung




Qualitäts-Schulden

Schlechte Code-Qualität = Kredit

Code-Review + Korrektur = Rückzahlung

Weiterentwicklung trotz Schwächen

= Zinszahlungen

können überwältigend werden!

Nichts tun = "absaufen"

Ad-hoc-Maßnahmen sind

keine Dauerlösung


More

Seminare zum Thema, z. B.

Java Effective

Java Software Testing

http://javaeeblog.wordpress.com/

http://expertenkreisjava.blogspot.de/

[email protected]

@dirkweil

[email protected] Gute Zeilen, schlechte Zeilen 46

Technology

Gute zeilen, schlechte zeilen - Regeln für wartbare Software