Donnerstag, 3. Juni 2010

CCD bewusst verletzen

dotnetpro ccd stempel2 almost half size banner Das ist mein erster Blog-Artikel. Motiviert dazu wurde ich durch Ralf Westfals Ausschreibung für ein kostenloses CCD-Praktikum unter http://www.prodevcollege.de/ccd-praktikum-ausschreibung.html. Aufgabe ist es, einen Artikel über einen CCD-Baustein zu schreiben.

Ich möchte euch vorstellen, wie ich vor kurzem im Rahmen eines großen Refactorings bewusst ein CCD-Prinzip gebrochen habe und dadurch erst eine elegante Lösung möglich wurde. Der eine oder andere wird sich nun fragen wie das mit CCD zusammenpasst. Aber alles der Reihe nach...

Um was es eigentlich geht

Mein Kunde bietet ein modulares Steuerungssystem für die industrielle Automatisierungstechnik an. Hiermit kann man Signale von Sensoren (Eingangsinformationen) in einer CPU logisch verknüpfen und deren Ergebnisse Aktoren (Ausgängen) zuweisen. Für eine Integration in andere Steuerungssystem / Netzwerke bietet er Gateways an, die der CPU Signale (“Network-to-CPU”, kurz “N2C”) bzw. Signale aus der Steuerung dem Netzwerk zur Verfügung stellen (“CPU-to-Network, kurz “C2N”).

CPU-to-Network
Jedes Gateway hat für CPU-to-Network und Network-to-CPU maximal 50 Bytes zur Verfügung, die man auf einer Konfigurationsseite bearbeiten kann. Per Drag & Drop zieht man die Bytes der CPU auf einen Byte-Slot eines Byte-Containers (“Fieldblock” genannt) des Gateways. Die Byte-Slots nennt man Fieldblock-Bytes oder kurz "FBByte".

Meine Aufgabe bestand nun unter anderem darin, die Komponente, die die Fieldblöcke visualisiert, für neue Anforderungen umzubauen.

Blick auf den Ist-Stand

In der existierenden Code-Basis gab es für ein FBByte eine Struct namens "FBByteLocation" mit zwei Properties: 1) der Offset des FBBytes und 2) ein Flag, ob es sich um ein Byte für CPU-to-Network oder Network-to-CPU handelt. Letzere Information ist wichtig, weil für beide Routing-Richtungen Daten unterschiedlich formattiert in der UI angezeigt werden müssen.

Da es mehrere verschiedene Gateway-Module gibt, sind die angezeigten Texte nicht nur von C2N / N2C abhängig sondern vom Modul selbst.

Aus diesem Grund gab es ein Interface, das von jedem Gateway implementiert wurde:

interface IByteProvider
{
   string GetFormattedOffset(FBByteLocation fbByte);
   ...
}

Typisch für die Implementierung jedes ByteProviders war, dass in den implementierten Funktionen eine Fallunterscheidung für C2N/N2C war:

if (fbByte.IsCpuToNetwork)
{
   ...
}
else
{
   ...
}

Das hat den Code schwer zu lesen und unnötig kompliziert gemacht. Solche Interfaces und Implementierung gab es für verschiedene Aspekte, d.h. sehr viele Stellen waren mit Fallunterscheidungen gespickt.

Das wäre noch gar nicht so schlimm und würde einen Umbau nicht unbedingt rechtfertigen. Es ist aber so, dass sich C2N und N2C in vielerlei Hinsicht unterscheiden. Funktionen, die für C2N Sinn machen, dürfen für N2C nicht aufgerufen werden und werfen eine Exception oder geben einen Dummy-Wert zurück.

Vorüberlegungen

Es war offensichtlich, dass hier nicht die richtigen Abstraktionen gewählt waren. Da es mir auch an anderer Stelle während des Refactorings viel Arbeit sparen würde, war der richtige Zeitpunkt gekommen, dieses unschöne Konstrukt aufzubrechen. Die naheliegende Lösung war, jeweils für C2N und N2C ein eigenes, spezialisiertes Interface an die UI-Komponente zu geben. Innerhalb der jeweiligen Implementierung ist dann keine Fallunterscheidung mehr notwendig.

Was aber tun mit der bisherigen Struct ByteLocation? Das Flag ist nun unnötig und eigentlich handelt es sich daher nur noch um den Offset des Bytes. Ich hätte daher das Interface wie folgt definieren können:

interface ICpuToNetworkByteProvider
{
   string GetFormattedOffset(int fbByteOffset);
   ...
}

Das hätte absolut Sinn gemacht, aber: ich war mir nicht 100% sicher, ob ich das Flag nicht doch noch gebrauchen könnte. Falls das der Fall wäre, müsste ich etliche Codestellen anpassen, um aus dem int wieder ein FBByteLocation zu machen.

Diese Gedankenspiele verstoßen ganz klar gegen das YAGNI-Prinzip ("You ain't gonna need it"). Dieses Prinzip sagt aus, dass Code, der nicht unmittelbar gebraucht wird, nicht implementiert werden soll.

Obwohl mir das bewusst war, wollte ich mich damit nicht abfinden und habe implizite Konvertierungsoperatoren zu der Struct hinzugefügt, damit eine Konvertierung von int nach FBByteLocation und umgekehrt möglich ist. Damit kann man die GetFormattedOffset()-Funktion auch mit einem int aufrufen. Falls sich meine Befürchtungen am Ende des Refactorings doch als unbegründet herausstellen, könnte ich mit relativ wenig Aufwand FBByteLocation wieder mit einem int ersetzen.

Umsetzung

Während der Umsetzung haben sich zunächst noch keine Vorteile meiner Entscheidung gezeigt. Nach einer Weile ist mir aber aufgefallen, dass ich folgenden kleinen Code-Schnipsel, der die Bits eines Bytes iteriert, schon mehrfach im Code gesehen habe:

for (int offsetWithinByte = 0; offsetWithinByte < 8; ++offsetWithinByte)
{
   int bitOffset = fbByte.Offset * 8 + offsetWithinByte;
   ...
}

Das war ein perfekter Kandidat für ein Refactoring! Ich habe eine neue Struct "FBBitLocation" angelegt mit zwei Properties OffsetWithinByte und Offset. Dazu eine Property in FBByteLocation:

public IEnumerable Bits
{
   get
   {
      for (int offsetWithinByte = 0; offsetWithinByte < 8; ++offsetWithinByte)
         yield return new FBBitLocation(this, offsetWithinByte);
   }
}

Damit konnte der ursprüngliche Code wie folgt umstrukturiert werden:

foreach (FBBitLocation fbBit in fbByte.Bits)
{
   ...
}

Das sieht auf den ersten Blick nur nach einer minimalen Verbesserung aus. Sie hat aber wesentliche Vorteile:
  • Code-Duplikation, auch wenn er noch so minimal ist, sollte niemals zugelassen werden.
  • man sieht auf den ersten Blick, was in dieser Schleife iteriert wird. Die Les- und Wartbarkeit des Codes steigt damit.
  • wir bleiben innerhalb des Codes in Begriffen der Problemdomäne. Auch das steigert die Lesbarkeit.
  • die Abstraktion FBByteLocation drückt aus, was ein int nicht kann: die Beziehung zu seinen Bits - ein Byte enthält 8 Bits.
  • als Entwickler sieht mit Intellisense, über welche Eigenschaften und Funktionen ein FBByteLocation verfügt und kann deshalb ohne zu überlegen diese Funktionalität benutzen.
  • die Bits-Property kann mit LINQ verwendet werden.

Während der Überlegungsphase habe ich an so eine Verbesserung noch nicht gedacht. Nun begann ich, auch für die Fieldblöcke eine Abstraktion einzuführen. Ich führte z.B. eine Property ein, mit der man die Bytes eines Fieldblocks iterieren konnte, oder eine Funktion, die prüft ob ein Byte zu dem Fieldblock gehört usw. FBByteLocation bekam noch eine Funktion GetBit(int offsetWithinByte). Damit baute ich an dieser Stelle ein DDD-Model im Miniaturformat auf. Nach und nach passte ich die Schnittstellen an die neuen, primitiven Datentypen an und habe alte und umständliche Konstrukte mit den Eigenschaften und Funktionen von FBLocation, FBByteLocation und FBBitLocation ersetzt.

Die Kontrakte wurden sicherer: die API kann jetzt nicht mehr falsch verwendet werden, weil man an der Funktionssignatur erkennen kann, dass ein Fieldblock Byte erwartet wird. Ein int ist hierfür zu anonym. Bei den Bits ist dies besonders auffällig: einer Variablen int bitOffset sieht man nicht an, ob sie den absoluten oder relativen Offset (zum zugehörigen Byte, d.h. 0-7) angibt. In FBBitLocation dagegen ist beides enthalten bzw. abstrahiert.

Ein weiterer positiver Aspekt ist, dass ich effektiver Debuggen konnte. Ich überschreibe generell ToString(). Ein FBLocation wird im Debugger jetzt als "FieldBlock 0-49", ein FBByteLocation als "FBByte 13" und ein Bit als "FBBit 5.0 (abs. 40)" angezeigt.

Fazit

Die Code-Verbesserungen haben sich bezahlt gemacht: viele Code-Stellen sind jetzt viel einfacher zu verstehen. Neue Funktionalität konnte ich schneller einbauen. Die API enthält jetzt Begriffe der Problemdomäne (siehe hierzu auch http://ralfw.blogspot.com/2010/06/die-ubiquitous-language-konsequent.html) und kann damit fehlerfreier benutzt werden.

Letztlich hat sich herausgestellt, dass ich das Flag für die Unterscheidung von CPU-to-Network und Network-to-CPU tatsächlich nicht mehr gebraucht habe. Hätte ich das vorher gewusst, hätte ich vielleicht das YAGNI-Prinzip befolgt und FBByteLocation entfernt. Ob ich während der Umsetzung dann auf die Idee gekommen wäre, diese Abstraktion wieder einzuführen - who knows?

Dieses Beispiel zeigt, dass man die CCD-Prinzipien nicht reflexartig und dogmatisch anwenden sollte. Denn durch die initiale Verletzung von YAGNI wurde es erst möglich, dass die Codebasis im Laufe vieler kleiner Änderungen immer besser geworden ist und am Ende über die positiven Aspekte anderer CCD-Werte verfügte (DRY, KISS, Principle of least astonishment).

CCD ist deshalb meiner Meinung nach ein nützliches Framework mit hilfreichen Richtlinien, ersetzt aber nicht den gesunden Menschenverstand eines Entwicklers.

Keine Kommentare:

Kommentar veröffentlichen