Design for Testability ist auch Design for Flexibility. Deshalb liebe ich TDD. Allerdings bin ich dabei immer wieder über einen Problemfall gestolpert: die Prüfung des Zustands eines system under test (SUT). Als Beispiel hier ein typischer (nicht unbedingt optimaler) Ansatz bei der bekannten und beliebten KataPotter.
Viele denken bei der Implementierung der Preisberechnung für die KataPotter an einen Warenkorb, den es zu füllen gilt, um dann den Preis unter Anwendung der Rabatte zu berechnen. Sie definieren eine solche Klasse dann z.B. so:
class KataPotterShoppingBasket
{
private Dictionary<int, int> books = new Dictionary<int, int>();
public void AddBook(int bookId)
{
//...
}
public decimal Total
{
get
{
//...
}
}
}
Ein zugehörige Test scheint zunächst einfach:
[TestFixture]
public class testKataPotterShoppingBasket
{
[Test]
public void Enthält_ein_Buch()
{
var sut = new KataPotterShoppingBasket();
sut.AddBook(1);
//?
}
}
Aber, ach, ohje… Wie prüft man denn nun, ob das hinzugefügte Buch auch im Warenkorb angekommen ist? AddBook() ist ein Kommando und verändert den Zustand des Warenkorbs. Wie kann man also ganz allgemein diesen Zustand in Tests prüfen?
Zugriffsroutinen für Zustand?
Naheliegend ist die Prüfung von Zustand durch direkten Zugriff auf den Zustand. Dafür muss das SUT nur einen solchen Zugriff anbieten. Für einen üblichen Warenkorb mag das der Fall sein. Da gibt es nicht nur ein AddBook(), sondern sicher auch noch ein GetBook() oder eine ganze Collection von Books, über die man prüfen kann, ob eine Zustandsveränderung wirksam war.
Der KataPotter-Warenkorb braucht solche Zugriffsroutinen aber nicht. Warum sollten sie also nur für den Test eingeführt werden? Ich halte nichts davon, weil sie das Interface des system under test aufblähen. Sie haben keinen Wert für die Domäne, sind in der Intellisense-Liste der Eigenschaften und Methoden aber immer sichtbar. Da hilft auch die Deklaration als intern nicht viel. Das wäre falsch verstandenes Design for Testability.
Wenn Sie die KataPotter so wie oben gezeigt angehen sollten, widerstehen Sie der Versuchung, dem Warenkorb z.B. eine Count-Eigenschaft zu geben, um den Test zum Laufen zu bekommen.
Solch ein Design wäre dann zwar irgendwie testbarer, aber die Flexibilität würde sich nicht erhöhen. Sie hätten nicht mehr das minimal Notwendige getan. Außerdem würde der Test dann zweierlei testen. Das ist zu vermeiden! Er würde AddBook() und Count prüfen. In beiden könnten ja Fehler stecken.
AddBook() und Count wären sogar gegenseitig abhängig in Bezug auf Tests. Sie könnten auch Count nicht zuerst testen, denn dazu müssten Sie Zustand aufbauen. Wäre das nicht der Fall, ist die Nutzung von mehreren SUT-Funktionalitäten in einem Test nicht so schlimm. Dann können Sie die in separaten Tests vorweg prüfen.
Ergebnis: Vermeiden Sie, spezielle Zugriffsmethoden für Zustand nur zum Zweck des Testens einzurichten. Allemal, wenn sie sie nicht wirklich isoliert testen können.
Zustandstest durch Ableitung?
Auf Zustand können Sie aber nicht nur von außen zugreifen, einfacher ist es von innen. Eine Alternative zu Zugriffsmethoden wäre daher die Ableitung einer Klasse vom SUT. Deren Zustand wäre ja der des Warenkorbs. Dazu müssten Sie nur die Sichtbarkeit des Zustands ändern:
class KataPotterShoppingBasket
{
protected Dictionary<int, int> books = new Dictionary<int, int>();
...
}
Eine Ableitung kann dann “gefahrlos” eine Zugriffsmethode anbieten. Die “verunreinigt” die Domänenklasse nicht:
class TestableShoppingBasket : KataPotterShoppingBasket
{
public int Count { get { return base.books.Count; } }
}
Wenn Sie im Test die abgeleitete Klasse als SUT instanzieren, haben Sie also Testbarkeit “ohne Reue”:
[Test]
public void Enthält_ein_Buch()
{
var sut = new TestableShoppingBasket();
sut.AddBook(1);
Assert.That(sut.Count, Is.EqualTo(1));
}
Mit einer Ableitung müssen Sie nicht einmal Design for Testability betreiben. Und die Flexibilität wird auch nicht eingeschränkt. Eine scheinbar ideale Lösung, oder? Der Eingriff, private Variablen auf protected statt private zu setzen, ist minimal. Und dass dieser Ansatz an abgeschlossenen (sealed) Klassen scheitert, ist auch unbedeutend.
Dennoch will mir eine Ableitung vom eigentlichen SUT nicht recht schmecken. Nein, das liegt nicht daran, dass ich Ableitungen für generell überbewertet halte. Hier wäre ja sogar das Liskov Substitution Principle eingehalten. Den Aufwand für die Ableitungsklasse finde ich auch nicht zu hoch.
Nein, es etwas anderes, das mich stört…
Zustand als Abhängigkeit
Ich glaube, mich stört, dass Zustand hier eine Extrawurst gebraten bekommt. Zustand wird nicht seiner Natur nach behandelt. Denn Zustand ist eine Abhängigkeit!
Üblicherweise denken wir bei Abhängigkeiten an Funktionalität. Der Warenkorb könnte z.B. von einer Preisberechnungsfunktionalität abhängen:
Dann fänden wir es ganz natürlich, diese Abhängigkeit in den Warenkorb zu injizieren:
class KataPotterShoppingBasket
{
private IPriceCalculator calc;
private Dictionary<int, int> books = new Dictionary<int, int>();
public KataPotterShoppingBasket(IPriceCalculator calc)
{
this.calc = calc;
}
...
Aber was ist jetzt das private Feld calc? Es gehört genauso zum Zustand des Warenkorbs wie books. Der einzige Unterschied besteht darin, dass calc von einem selbstdefinierten Typ ist und book einen .NET Fx Typ hat.
Darüber hinaus glauben wir dann aber irgendwie noch eine größere “Intimität” zwischen book und z.B. AddBook() zu spüren. Die Inhalte von book werden mittels der Warenkorbmethoden verändert, während calc konstant bleibt. book ist eben “wahrer” Zustand und deshalb anders zu behandeln als calc.
Doch warum ist die Abhängigkeit von einem Preisberechner denn offengelegt? Warum wird der injiziert? Weil das den Warenkorb isoliert testbar macht. Sie können ihn unter Injektion einer Attrappe auf die Probe stellen. Dazu kommt, dass damit die Preisberechnung auch noch flexibler austauschbar wird; ein schöner Nebeneffekt.
Wenn nun jedoch bessere Testbarkeit zur IoC für funktionale Abhängigkeiten geführt hat, warum sollte sie dann nicht auch bei Abhängigkeiten von Zustand zum Einsatz kommen? Ich sehe da kein gewichtiges Gegenargument. Das widerspräche auch nicht der Objektorientierung. Nach Injektion ist Zustand ja weiterhin in einem Objekt entsprechend dessen Zugriffsmethoden verborgen.
Wenn wir von Abhängigkeiten sprechen, sollten wir also konsequent sein. Zustand (state) wie Funktionalität (behavior) sind zwei Seiten derselben Medaille Abhängigkeit:
Aus meiner Sicht sieht die Lösung des Ausgangsproblems deshalb so aus:
class KataPotterShoppingBasket
{
private readonly PriceCalculator calc;
private readonly Dictionary<int, int> books;
public KataPotterShoppingBasket(Dictionary<int, int> books,
PriceCalculator calc)
{
this.calc = calc;
this.books = books;
}
...
Dem erweiterten Warenkorb injizieren Sie nicht nur die Abhängigkeit von einer Preisberechnungsfunktionalität, sondern auch einen initialen Zustand:
[Test]
public void Enthält_ein_Buch()
{
var books = new Dictionary<int, int>();
var sut = new KataPotterShoppingBasket(
books,
null);
sut.AddBook(1);
Assert.That(books.Count, Is.EqualTo(1));
}
Das halte ich für genauso konsequent wie einfach zu verstehen. Etwas systematisiert lautet dann das Muster:
- Abhängigkeiten einer Klasse zusammenfassen in einer Abhängigkeitsklasse. Das gilt für Zustände wie Funktionalität, von denen eine Klasse abhängt.
- Während Tests die dafür minimal nötigen Abhängigkeiten mit einer Instanz der Abhängigkeitsklasse injizieren.
- Erwartungen an Zustandsänderungen nach Ausführung von Kommandos auf dem SUT prüfen anhand der injizierten Abhängigkeitklasseninstanz.
Am Beispiel des Warenkorbs sähe das so aus:
class KataPotterShoppingBasket
{
internal class Dependencies
{
public readonly IPriceCalculator calc;
public readonly Dictionary<int, int> books = …
}
private readonly Dependencies dependencies;
public KataPotterShoppingBasket(Dependencies dependencies)
{
this.dependencies = dependencies;
}
...
Der Test wäre sogar noch ein wenig klarer, finde ich:
[Test]
public void Enthält_ein_Buch()
{
var dependencies = new KataPotterShoppingBasket.Dependencies();
var sut = new KataPotterShoppingBasket(dependencies);
sut.AddBook(1);
Assert.That(dependencies.books.Count, Is.EqualTo(1));
}
Diesem Verfahren steht auch die “normale” Dependency Injection nicht im Wege. Sie würde es sogar verbergen. Dazu bräuchte die Abhängigkeitsklasse nur einen ausgewiesenen “Kanal” für die Injektion der Funktionalität, von der der Warenkorb abhängig ist, z.B. so für Unity als DI Container:
public class Dependencies
{
[Dependency]
public IPriceCalculator calc {get; set;}
public readonly Dictionary<int, int> books = …
}
Zusätzlich müsste im Mapping auch die Abhängigkeitsklasse aufgeführt werden. Aber das ist kein sonderlicher Mehraufwand. Das ließe sich womöglich sogar automatisieren.
Fazit
Im Sinne eines systematischen Vorgehens beim Test und beim Design finde ich es sehr überlegenswert, Abhängigkeiten breiter als bisher zu fassen. Funktionseinheiten sind nicht nur von anderen Funktionseinheiten abhängig, sondern auch von Zustand.
Wenn dann Abhängigkeiten injiziert werden sollen, um bessere Testbarkeit zu erreichen, dann gilt das nicht nur für Funktionseinheiten, sondern auch für Zustand.
Gerade bei TDD sinkt dadurch – so mein Empfinden – der gedankliche Aufwand. Bei jedem Kommando und jeder Abfrage, die getestet werden, müssen Sie sich dann einfach nur die Frage stellen, welcher Zustand dabei eine Rolle spielt. Dann bauen Sie vor dem Test Ausgangszustand speziell für den Testfall zusammen (arrange) und injizieren ihn, rufen das SUT (act) auf und prüfen anschließend (bei Kommandos) den neuen Zustand (assert). Fertig.
Ich werde in der nächsten Zeit mal probeweise konsequent so vorgehen. Mal schauen, wie sich das anfühlt. Ich bin optimistisch, dass es ein gutes Gefühl sein wird. Weniger Zweifel, weniger Nachdenken, also höhere Produktivität.
14 Kommentare:
Das ist alles so interessant! Ich denke, so langsam wird es Zeit für ein Buch, Ralf. :-)
In aller Vorsicht (bin Anfänger bei Unit Tests) die Frage: Was wäre, wenn AddBook einen Rückgabewert hätte, der die Anzahl der Bücher zurückgibt?
Hallo Ralf,
Sehr spannender Gedanke. Genau das stört mich immer.
// Arrange
// Act
// Assert.... und das Assert hier ist eigentlich schonwieder ein ACT, also quasi ein Integrationstest, wenn ich es über eine Count Property teste.
Vielleicht ist deine Lösung ein eleganter Umweg. So richtig süss schmeckts mir aber noch nicht... wieso kann ich nicht in Worte fassen, bin noch misstrauisch, werds aber mal versuchen.
grüsse
@Georg: Warum sollte AddBook() einen Rückgabewert haben? Das ist für die Problemdomäne unnötig. Und genau darum geht es: Keine Zugriffsmethoden einführen, die die Problemdomäne nicht motiviert.
Für Klassen, die sich auf die konkrete Domäne beziehen, finde ich die Lösung sehr toll.
Aber was ist mit der Unmenge an Helfer Klassen die im Hintergrund werkeln?
Die möchte ich auch verwenden können ohne einen DI-Container wie Unity zu verwenden oder die Zustände manuell im ctor zu übergeben. So ganz glücklich fühle ich mich auch nicht, muss mich da Laurin Stoll anschließen.
@Sebastian: Im Produktionscode musst du den Dependency-Ctor gar nicht verwenden. Allemal für Zustand nicht.
Der Dependency-Ctor ist eine "Revisionsklappe". Die benutzt du, wenn du testest. Der kann auch internal sein, selbst wenn die Klasse public ist.
Du kannst auch zweierlei solcher DI-Ctors haben:
-einen für den Produktionsbetrieb, wo du nur behavior dependencies reinreichst
-einen für Tests, wo du behavior und state dependencies reinreichst
-Ralf
Schade, dass ich Euren Blogg erst heute entdeckt habe. Vielen Dank für den interessanten und nützlichen Artikel!
Hallo Ralf,
ich bin ganz bei dir, wenn es darum geht die Problemdomäne nicht unnötig aufzublähen, daher finde ich den Ansatz ganz interessant.
Eine Sache gefällt mir aber nicht (vielleicht habe ich es aber auch nicht richtig verstanden).
Beim injizieren eines Zustands über den Konstruktor, geht da nicht ein wenig die Datenkapselung hopps, wenn es wie oben beschrieben umgesetzt wird?
Normalerweise würde ich mir im Konstruktor eine Kopie der Bücher erstellen, sodass ich alleine, die Kontrolle behalte.
@Andi: Die Datenkapselung geht nicht hopps. Der Ctor für die Zustandsinjektion ist ja nur für Testzwecke da. Normaler Client-Code benutzt andere Konstruktoren. Deshalb ist der Ctor auch internal.
-Ralf
dann sind alle Abhängigkeiten immer in internen klassen und nicht mehr in der eigentlichen Klasse?
Das würde eine komplett neue Struktur beim Programmieren bedeuten wenn ich das richtig sehe. Es wird quasi das "this" durch "dependencies" ersetzt.
Mfg Tarion
@Tarion: Wäre die "Struktur beim Programmieren" komplett neu? Nö, würd ich nicht sagen. Aus this.myfield würde this.state.myfield oder this.dependencies.myfield oder this.dependencies.state.myfield. Das find ich jetzt nicht sooo anders als bisher.
-Ralf
Hallo Herr Westphal!
Grundsätzlich folge ich begeistert Ihrem Gedankengang, nur ein "komisches" Gefühl bleibt auch bei mir stehen.
Ein paar Gedanken die bleiben
a) die Lösung ist gut. sollte/darf aber wahrscheinlich nicht zwangsweise, unbedacht und überall angewendet werden.
b) bei Properties, bei welchen ich bisher die "kurze" Schreibweise mit {get;set} verwendet habe erhöht sich der Aufwand/Fehlerrisiko weil ich explizite Getter/Setter schreiben muss
c) ok wenn es einer Property Bedarf dann könnte man sich das Feld auf der private class sparen => Mischmasch welche Felder stehen direkt im this und welche im this.Dependencies
c) Der einzige Zweck einer Methode (ohne out-Parameter) kann nur eine Zustandsänderung des Objektes sein.
Diese Zustandsänderung MUSS zwangsläufig irgendwo (oftmals auch indirekt) ihre Auswirkung finden (public!), sonst würde das Ganze keinen Sinn machen.
Im Einkaufskorb-Beispiel sehe ich dafür zwei Möglichkeiten
c1) die Wahrscheinlichkeit dass spätestens in der 2ten Iteration eine Property Count im Warenkorb dazukommt liegt laut meiner Erfahrung bei 99%
c2) Man könnte den injizierten PriceCalculator verwenden um die Zustandsänderung zu testen
oder anders formuliert:
Je höher die Komplexizität der Klasse desto mehr Nutzen Ihrer vorgeschlagenen Lösung sehe ich (besseres Kosten/Nutzen Verhältnis), bei einfachen Klassen habe ich ein Bauchgefühl welches sagt "zu aufgeblasen"
andererseits deutet eine "komplexe Klasse" schon wieder auf einen Designfehler hin der schnellstens behoben gehört....
Alles Gute fürs Neue Jahr
Werner Mairl
@Werner Mairl: Zu Ihren Punkten:
a) "Zwangsweise" klingt nicht schön. Aber warum denn auch gleich in Zwängen denken? Warum nicht "Muster" oder "Richtlinie"?
Niemand hat GOTO verboten - und doch setzen wir es nicht mehr ein. Nicht aus Zwang, sondern aus Einsicht.
Genauso hier: Für mich ist es eine Einsicht, dass Zustand eine Abhängigkeit ist, die - für Testzwecke - genauso injiziert werden sollte wie eine funktionale Abhängigkeit.
Niemand zwingt uns dazu, DI zu benutzen. Und doch tun wir es. Wir sehen ein, dass wir ansonsten keine vernünftige Testbarkeit hinbekommen. Um nichts anderes geht es mir bei der Injektion von Zustand.
b) Hier weiß ich nicht, worauf Sie sich beziehen. Wenn es Properties gibt, muss ich Zustand nicht unbedingt injizieren, um ihn setzen/abfragen zu können. Mir geht vielmehr eben darum, dass kein Zugriff von außen auf den Zustand existiert, der in einem Test relevant ist.
d) Zustandsänderungen müssen nicht notwendig öffentlich sichtbar sein. Sie fließen sicherlich ein in andere Methodenaufrufe, aber deshalb kann ich den Zustand noch lange nicht gezielt setzen/prüfen.
d1) Da schauen Sie in die Glaskugel. Solche "Erfahrung" aus Testgründen in einer Count-Property zu manifestieren halte ich für einen Widerspruch zum YAGNI-Prinzip. Damit treten Sie ein Fenster ein, das zu weiteren eingetretenen Fenstern führt... ("broken window Theorie").
d2) Um Zustand zu testen, möchte ich eben keine anderen SUTs benutzen. Denn dann teste ich ich nur eine Funktionalität.
Insgesamt verstehe ich nicht, dass es ein Problem ist, Zustand hinein zu reichen. Wie gesagt: dazu braucht es nur einen internen ctor. Es geht ja eben nicht (!) um die Änderung einer öffentlichen Schnittstelle.
Für mich steht die saubere Testbarkeit weit vor dem minimalen Mehraufwand einer Zustandsklasse.
-Ralf Westphal
Kommentar veröffentlichen