Heute will ich mal wieder aus einem Beispiel aus meinem Projektalltag ansprechen.
Es geht um Kalendereinträge in SharePoint. Wir haben 4 verschiedene Typen von Einträgen: Urlaub, Krank, Außer Haus, Im Haus. Im vorliegenden Fall müssen die Typen Urlaub und Krank exportiert werden. Generell ist es so, dass alle Typen bis auf 1-2 Felder die gleichen Eigenschaften besitzen. Ergo arbeite ich mit Vererbung. Die Base Class nennt sich CalendarEntry, von der die zwei Kindsklassen HolidayEntry und SicknessEntry erben (reine Data Transfer Objects).
Da ich die Daten untypisiert von SharePoint erhalte, habe ich mir eine generische Factory gebaut, die mir die 2 besagten Klassen instanziiert und füllt. Das Interface dazu sieht wie folgt aus:
1: public interface ICalendarEntryFactory<T> where T:CalendarEntry, new()
2: {
3: IEnumerable<T> Build(SPListItemCollection calendarEntries);
4: }
Die implementierende Klasse enthält also eine Methode namens Build, die mir aus den Kalendereinträgen von SharePoint eine Liste von typisierten Objekten (IEnumberable deswegen, weil es allgemeiner ist als List<T>) zurückgibt. Um welche Objekte es sich handelt, die die Factory erzeugt, wird beim Instanziieren der Factory angegeben, allerdings habe ich den Objekttyp auf CalendarEntry oder eine Ableitung davon eingeschränkt (über where T:CalendarEntry in Zeile 1). Außerdem muss das Objekt (also mein DTO), welches die Factory später erzeugen können soll, einen parameterlosen, öffentlichen Konstruktor haben. Dies gewährleiste ich über das new() in Zeile 1.
Die Klasse, die nun dieses Interface implementiert, sieht wie folgt aus:
1: public class CalendarEntryFactory<T> : ICalendarEntryFactory<T>
where T : CalendarEntry, new()
2: {
3: private readonly IRulesEngine<T> _rulesEngine;
4:
5: public CalendarEntryFactory(IRulesEngine<T> rulesEngine)
6: {
7: _rulesEngine = rulesEngine;
8: }
9:
10: public IEnumerable<T> Build(SPListItemCollection calendarEntries)
11: {
12: ...
13: }
14: }
Noch kurz einen Hinweis zu dem Konstruktorparameter rulesEngine in Zeile 5: Da DTOs niemals Logik enthalten sollten und die Factory, wie der Name schon sagt, lediglich die Funktion erfüllt, dass es Objekte erzeugen kann, benötige ich noch ein Regelwerk, welches die erzeugten DTOs prüft, ob diese korrekt sind. Nur, wenn diese korrekt sind, werden sie der Liste, welche in der Build-Methode zurückgegeben wird, hinzugefügt. Meiner Meinung ist das sehr sinnvoll, um eine stärkere Code Kohäsion zu erhalten. Das entspricht auch den Clean Code Prinzipien Single Responsibility Principle und Seperation of Concerns. Die RulesEngine ist wie man sieht ebenfalls generisch gehalten. Logischerweise mit dem gleichen Typparameter T!
Ein Beispiel, was ich unter anderem mit der RulesEngine prüfe: Ist der Urlaubseintrag vom Typ JAZ (= Jahresarbeitszeit, ohne näher zu erklären, was JAZ bedeutet), so handelt es sich um einen Eintrag, der nicht in das Lohnprogramm exportiert werden darf.
Aber zurück zu der Build-Methode, die mir generisch die Objekte erzeugen soll: Da die Objekte teilweise unterschiedliche Eigenschaften haben (nämlich die, die in den ableitenden Klassen selbst definiert sind), muss ich in der Build-Methode eine Fallunterscheidung machen. Der Code sieht wie folgt aus:
1: var entriesResult = new List<T>();
2: T entry;
3:
4: foreach (SPListItem calendarEntry in calendarEntries)
5: {
6: entry = new T();
7:
8: foreach (SPListItem calendarEntry in calendarEntries)
9: {
10: //erledige Zuweisungen, die bei allen Eintragstypen gleich sind
11:
12: if (entry.GetType().Equals(new HolidayEntry().GetType()))
13: {
14: //erledige spezielle Zuweisungen für den Eintragstyp Urlaub
15: }
16: else if (entry.GetType().Equals(new SicknessEntry().GetType())) {
17: //erledige spezielle Zuweisungen für den Eintragstyp Krank
18: }
19:
20: //erledige Anweisungen, die bei allen Eintragstypen gleich sind
21: if (_rulesEngine.Validate(entry)) entriesResult.Add(entry);
22: }
23: }
In Zeile 6 sieht man, warum ich bei meinem generischen Constraint dieses “New()” angegeben hatte: Ich muss zur Laufzeit von dem Typ, für den die Factory instanziiert wurde, ein Objekt erzeugen können. Die eigentliche Problematik tritt auf, wenn man die Fallunterscheidung durchführen will (Zeile 12 und 16):
Eine Prüfung auf den Namen des Typs wollte ich nicht durchführen, das ich dann den Namen hardcodiert als Zeichenfolge hätte hinterlegen müssen. Deshalb Instanziiere ich mir in den Zeilen 12 und 16 ein leeres Objekt und hol mir dessen Typ. Der Vorteil ist, dass Refactoring Tools und natürlich auch der Compiler das erkennen. Statt der If-Else-Verschachtelung wollte ich eigentlich eine Switch-Anweisung nehmen (macht natürlich v.a. dann Sinn, wenn man viele solcher Typen hat), allerdings ist hier das Problem, dass der Compiler eine Konstante in den Case-Anweisungen erwartet. Das ist leider nicht gegeben, da ja erst zur Laufzeit bekannt ist, was “entry.GetType()” eigentlich ist.
Peter Hallam, Entwickler bei MS, hatte den Hintergrund dazu bereits 2005 gebloggt. Das macht natürlich Sinn, dass man kein switch auf einen Typen anwenden kann.
Wer allerdings keines der genannten Szenarien aus zuvor erwähntem Blogeintrag bei sich in der Anwendung vorliegen hat, kann sich die Klasse aus dem Blogeintrag switching on types hernehmen, sodass er nicht mit if-else-Verzweigungen arbeiten muss.
Mit Tag(s) versehen: Clean Code
Uli Armbruster 
Versteh ich nicht. Warum muss denn die ICalendarEntryFactory generisch sein? Wie benutzt man die denn? Etwa so:
var factory = new CalendarEntryFactory(); ??
Dann würde die Aufzählung aber immer nur von Typ HolidayEntry und nie vom Typ SiknessEntry sein.
LikeLike
Hey Carsten,
der Aufruf wäre wie folgt:
var holidayEntryFactory = new CalendarEntryFactory(Of SicknessEntry)(…)
Da aus den Kommentaren spitze Klammern gelöscht werden, habe ich jetzt „(Of SicknessEntry)“ statt spitze Klammern geschrieben.
Danach kann die Build Methode dir entsprechend Krankheitstage generieren.
Allerdings wäre die bessere und sauberere Variante die, dass man das Abstract Factory Pattern einsetzt, statt nur eine Factory zu schreiben.
LikeLike
Alles klar. Aber warum muss denn das generisch sein?
LikeLike
Macht das Austauschen des Codes einfacher und der wesentliche Punkt: Don’t repeat yourself -> Code nicht doppelt schreiben
LikeLike
Eine Schnittstelle wird dann eingesetzt, wenn man mindestens zwei Implementierungen kennt oder antizipieren kann. Der einzige Anwendungsfall den ich mir denken könnte, wäre dass Du zwei Factories (eine für HolidayEntry und die andere für SicknessEntry) implementieren würdest. Das wäre auch gut für Deine IRulesEngine (bräuchte man nicht mehr).
Das Problem was ich mit Deinem Beitrag habe ist, dass das Problem sehr trivial ist (Konvertieren von Instanzen eines Typen in Instanzen anderer Typen), Du aber einen riesigen Overhaed (Factories, Interfaces, Generics) erzeugst. Diesen Overhead zu vermeiden, ist eben auch ein wesentlicher Aspekt des DRY Prinzips.
LikeLike
„Eine Schnittstelle wird dann eingesetzt, wenn man mindestens zwei Implementierungen kennt oder antizipieren kann.“ <- Ich bin nicht sicher, worauf du dich beziehst. Das Programm an sich könnte man als Schnittstelle bzw. Converter bezeichnen. Falls du dich aber in deinem Kommentar auf die Factory beziehst, so kann ich dem nicht zustimmen. Eine Schnittstelle definiert einen Kontrakt an den Grenzen zweier Systeme. Es kennt in der Regel nur eine Implementierung, die andere Seite ist eine Black Box (vgl. http://de.wikipedia.org/wiki/Schnittstelle). Wenn es keine kennt, würde ich auch eher von einem Converter sprechen.
"zwei Factories"
Wie gesagt: Heute würde ich das Abstract Factory Pattern anwenden, sodass es zwei Factory Implementierungen gemäß der gemeinsamen Schnittstelle gäbe. Die RulesEngine bräuchte man weiterhin, da es sich um einen separaten Aspekt – in dem Fall Validierung – handelt. Dementsprechend gilt hier Seperation of Concerns.
"sehr trivial":
Trivial ist relativ und trivial bedeutet nicht, dass der Code damit nicht den gängigen Prinzipien folgen sollte. Das sieht man beispielsweise bei Coding Dojos an den Katas wie z.B. FizzBuzz (vgl. http://codingdojo.org/cgi-bin/wiki.pl?KataFizzBuzz). Auch der Aussage, dass DRY bedeutet Overhead zu vermeiden, stimme ich nicht zu. DRY sagt, du sollst dich nicht wiederholen. Und um Overhead handelt es sich nicht, wenn ich ein lose gekoppeltes, gut skalierbares, einfach zu wartendes Programm umsetzen möchte. Wenn man sich bei kleinen Anwendungen nicht angewöhnt sauber zu entwickeln, wird es bei großen der Erfahrung nach auch nichts. Als Beleg kann ich auch hier nur wieder auf die immer mehr Einzug haltenden Coding Dojos in Firmen hinweisen!
LikeLike
Mit der Schnittstelle meine ich das Interface, in Deinem Falle die ICalendarEntryFactory. Die Schnittstelle soll ein bestimmtes Verhalten bei unterschiedlichen Implementierungen zuzusichern. Die Betonung liegt dabei auf unterschiedliche (also mehrere) Implementierungen. Nimm doch als Beispiel die Schnittstelle IEnumerable. Hiervon gibt es im .Net Framework sicherlich einige Dutzend verschiedene Implementierungen.
Da es in Deinem Fall keine weiteren Implementierungen gibt, reicht der „Vertrag“ durch die Signatur der konkreten Klasse als Definition der Schnittstelle aus. Sie gleichsam doppelt vorzuhalten (konkret und als Schnittstelle) und widerspricht daher dem DRY Prinzip.
Problematisch ist Dein Beispiel, weil es durch Ungeübte als elegant angesehen werden könnte, nur weil es vorgeblich nach CC Prinzipen entworfen wurde und halbwegs virtuos mit Generics umgeht.
LikeLike
Diese Aussage, dass es redundant wäre, wenn man ein Interface obwohl es nur eine Implementierung gibt, hatten wir mal sogar in großer Runde auf einem .NET User Group Treffen und das Ergebnis war: Nein, es ist nicht redundant. Allein schon aus einem ganz wesentlichen, in so gut wie jedem Fachbuch erwähnten Punkt: Niemals gegen konkrete Implementierungen binden, sondern gegen Abstraktionen! Bei mir gibt es prinzipiell keine Klassen mit öffentlichem Verhalten ohne ein entsprechendes Interface. Ausnahme sind Domänenobjekte, aber belassen wir es mal bei der Aussage.
Was man in dem obrigen Code nicht sieht, ist die Verwendung eines IoC Containers. Das hatte ich der Einfachheit hier nicht gezeigt. Allein deshalb war für mich die Verwendung obligatorisch. Dadurch könnte ich jetzt auch Verhalten erweitern bzw. gänzlich austauschen, ohne meinen bestehenden Code anfassen zu müssen (= Einhaltung des Open Closed Principle). Könntest du in dem von dir vorgeschlagenen Fall nicht!
Zusammenfassend halte ich also konsequent OCP, Dependency Inversion (durch IoC), SoC und SRP ein. Somit sind schon 3 SOLID Prinzipien erfüllt. Während dein Vorschlag primär auf DRY abzielt, was im Vergleich zu den anderen Prinzipien imho weniger an Gewicht hat, mal ganz abgesehen davon, dass ich auch dieses in meiner Implementierung nicht verletzt sehe, weil es eben die Definition eines Kontrakts und die Implementierung für den Kontrakt ist, die ich auch zur Umsetzung des IoC Konzepts benötige.
„Problematisch ist Dein Beispiel, weil es durch Ungeübte als elegant angesehen werden könnte, nur weil es vorgeblich nach CC Prinzipen entworfen wurde und halbwegs virtuos mit Generics umgeht.“ <- Ich sehe das immer noch als elegant (Ausnahme das mit dem Abstract Factory Pattern, was ich bereits eingräumt habe), da ich deine Argumentation aus oben genannten Gründen nicht teile.
LikeLike
Dass man grundsätzlich nur gegen Interfaces entwickeln sollte, ist wirklich Unsinn. Selbst im .Net Framework sind viele Dinge sehr konkret implementiert. Man ist grundsätzlich dort flexibel, wo man Anforderungen kennt, die diese Flexibilität (und deren Overhead) rechtfertigen. Überall dort, wo dies nicht der Fall ist, ist man so konkret wie irgend möglich.
Es geht mir darum, dass Du durch die Nutzung von Begrifflichkeiten den Eindruck erweckst, dass diese genauso umgesetzt werden. Nimm als Beispiel DDD: hier geht es darum, in seinem Modell möglichst nahe am Problem zu bleiben. In Deinem Fall ist das Problem das Konvertieren zwischen Kalendertypen aus Sharepoint. Die Tatsache, dass Du Dich hierzu eines Factory Musters bedienst, ist ein Detail der Implemtierung. Konkret bedeutet dies, dass Deine Klasse eher CalenderEntriesConverter (statt CalenderEntryFactory) hieße oder besser noch, da diese Konvertierung spezifisch für den SharePoint Kontext ist, SharePointCalendarEntriesConverter. Durch die Nutzung des Plurals wir zudem deutlich, dass hier Mengen von Einträgen konvertiert werden. Diese Zusammenhänge zwischen Modell und Problem herzustellen ist nach meinem Verständnis DDD.
Nimm als weiteres Beispiel das SRP: Es besagt doch, dass eine Klasse eine Verantwortung haben soll. Deine „Factory“ hat aber zum einen die Aufgabe zwischen den Typen zu Konvertieren und zum anderen eine Liste von Einträgen zu filtern. Es wäre besser dies zu trennen und einen Konverter zu erstellen und eine separate Zuständigkeit des Filterns (Vielleicht als Extension zum SPListItemCollectionType). Mit diesem Filter, wäre dann auch Deine „RulesEngine“ obsolet (am Namen kann man schon erkennen kann, dass was falsch ist, siehe DDD).
Beispiel IoC: IoC ist eine Technik, die Verhalten dynamisch austauschbar hält. Der Fokus liegt dabei auf dynamisch (bis hin zur Laufzeit). Es gibt sehr gute Beispiele in denen sich der Einsatz von IoC Techniken lohnt, Dein Beispiel scheint mir jedoch ein eher schlechtes zu sein, weil ihm etwas Wichtiges fehlt: die Dynamik: Ich gehe davon aus, dass sich die Logiken um die Konvertierungen der Kalendertypen selten ändern (eher schon die Menge der zu unterstützenden Typen). Wenn sie sich ändern, dann wird diese Fragestellung sicherlich bei Dir landen und Du (oder Deine Abteilung) wird mit der Umsetzung betraut. Da Ihr aber höchstwahrscheinlich den Source-Code der Anwendung besitzt, gibt es keine Notwendigkeit, hier etwas Dynamisches vorzusehen. Die Dynamik ist dann eher hinderlich, da der resultierende Code schwerer zu verstehen ist.
LikeLike