Re: Refactoring to DIP -> fachliches Problem

[Leba]

Jetzt ist mir gerade die Lösung selber eingefallen.

Man muss nur ein interface für die verschiedenen Calculators einführen...

Ahh, was für ein überflüssiger Post :S

Am Montag, 14. Januar 2013 23:33:11 UTC+1 schrieb Leba:

Hallo,

ich schreibe gerade an meiner Seminararbeit, in der es unter anderem um das Thema SOLID Principles geht.

Dafür habe ich mir eine kleine Beispielapplikation ausgedacht, um diese schrittweise nach den SP umzustrukturieren.

Dabei bin ich auf ein kleines Problem gestoßen.

Es geht um DIP mit Constructor Injection.

Vor dem "Refactoring to DIP" sieht der Code so aus:

class Program

{

    static void Main(string[] args)

    {

        Employee employee1 = new BackOfficeAssistant();

        Employee employee2 = new MarketingAssistant();

    }

}

public abstract class Employee

{

    public string Name { get; set; }

    public double MonthlyPay { get; set; }

    public abstract void PrintAnnualPayment();

}

public class MarketingAssistant : Employee

{

    public override void PrintAnnualPayment()

    {

        var paymentCalculator = new MarketingAssistantPaymentCalculator();

        Console.WriteLine("{0}: {1}", Name, paymentCalculator.CalculateAnnualPayment(this));

    }

}

public class BackOfficeAssistant : Employee

{

    public override void PrintAnnualPayment()

    {

        var paymentCalculator = new BackOfficeAssistantPaymentCalculator();

        Console.WriteLine("{0}: {1}", Name, paymentCalculator.CalculateAnnualPayment(this));

    }

}

public interface IPaymentCalculator

{

    double CalculateAnnualPayment(Employee employee);

}

public class MarketingAssistantPaymentCalculator : IPaymentCalculator

{

    private static double bonusInPercentage = 0.15;

    public double CalculateAnnualPayment(Employee employee)

    {

        return employee.MonthlyPay * 12 * (1 + bonusInPercentage);

    }

}

public class BackOfficeAssistantPaymentCalculator : IPaymentCalculator

{

    public double CalculateAnnualPayment(Employee employee)

    {

        return employee.MonthlyPay * 12;

    }

}

Daraus entsteht dies:

class Program

{

    static void Main(string[] args)

    {

        Employee employee1 = new BackOfficeAssistant(new BackOfficeAssistantPaymentCalculator());

        Employee employee2 = new MarketingAssistant(new MarketingAssistantPaymentCalculator());

        // fachlich nicht sinnvoll

        Employee employee3 = new MarketingAssistant(new BackOfficeAssistantPaymentCalculator());

    }

}

public abstract class Employee

{

    protected readonly IPaymentCalculator paymentCalculator;

    public string Name { get; set; }

    public double MonthlyPay { get; set; }

    public Employee(IPaymentCalculator calcualtor)

    {

        this.paymentCalculator = calcualtor;

    }

    public void PrintAnnualPayment()

    {

        Console.WriteLine("{0}: {1}", Name, paymentCalculator.CalculateAnnualPayment(this));

    }

}

public class MarketingAssistant : Employee

{

    public MarketingAssistant(IPaymentCalculator calculator)

        : base(calculator)

    {

    }

}

public class BackOfficeAssistant : Employee

{

    public BackOfficeAssistant(IPaymentCalculator calculator)

        : base(calculator)

    {

    }

}

public interface IPaymentCalculator

{

    double CalculateAnnualPayment(Employee employee);

}

public class MarketingAssistantPaymentCalculator : IPaymentCalculator

{

    private static double bonusInPercentage = 0.15;

    public double CalculateAnnualPayment(Employee employee)

    {

        return employee.MonthlyPay * 12 * (1 + bonusInPercentage);

    }

}

public class BackOfficeAssistantPaymentCalculator : IPaymentCalculator

{

    public double CalculateAnnualPayment(Employee employee)

    {

        return employee.MonthlyPay * 12;

    }

}

Das Problem ist, dass es nun möglich ist, einem MarketingAssistent ein BackOfficePaymentCalculator zu übergeben.

Wie kann man das am besten lösen?

Was man machen könnte, wäre eine Factory für den IPaymentCalculator einzuführen, die ein Dictionary mit Typenpaaren bekommt und dann für Jede Art von Employee die entsprechende Instanz des richtigen Typs produziert.

Aber irgendwie kommt mir das ein bisschen zu kompliziert für so ein einfaches Szenario vor.

Gibt es nicht vielleicht eine einfachere Möglichkeit, oder ist hier vielleicht grundsätzlich etwas verkehrt?

Würde mich über ein paar Anregungen sehr freuen! Danke.

[Ralf Westphal]

Ich finde nicht, dass dein Posting überflüssig war. Es ist vielmehr ein schönes Beispiel für Legacy Code, wie er ist. Und es ist ein noch schöneres Beispiel dafür, warum es überhaupt dazu kommt und wie Prinzipien zu Cargo Cult werden können. Sorry, hört sich jetzt vielleicht etwas hart an, ist ja aber nicht persönlich gemeint. Du verhältst dich eben typisch, finde ich. Das ist total verständlich - doch ich glaube, wir müssen genau aus diesem Verhalten raus.

Also, was ich meine:

Du präsentierst uns hier einfach Code. Der ist so, wie er ist. Ohne Geschichte.

Und du formulierst ein Ziel: DIP anwenden.

So ist es ganz häufig. Ein anderer präsentiert ein Problem und hat das Ziel, darauf die TPL oder den EF oder das Strategy Pattern anzuwenden.

Das Muster ist: das Mittel wird zum Ziel, zum Zweck erhoben. Der Bock wird zum Gärtner gemacht. Das halte ich für Cargo Cult: "Wenn ich X anwende, dann wird alles gut. Und das muss doch irgendwie gehen."

Aus dem Blick verloren wird dabei Kontext. Das ist Tunnelblick.

Ich versuche es mal mit einer Analogie:

Da steht einer am Straßenrand mit seinem Auto und kaputtem Motor. Du hältst an und er fragt dich, ob du dich mit Motoren auskennst. Dann beugt ihr euch beide über den Motor und fangt an zu tüfteln. Das dauert Stunden. Am Ende gratuliert ihr euch, es ist geschafft. Der Motor läuft wieder - aber der Geschäftstermin des Autofahrers ist geplatzt.

Der Tunnelblick hat nur noch den Motor gesehen. Der Motor war das Ziel eures ganzen Trachtens. Dabei sind Auto und Motor nur Hilfsmittel, um den Fahrer zu einem Geschäftstermin zu bringen.

Wenn dann der Motor streikt, ist es verständlich, kurz mal zu überlegen, ob man ihn wieder in Gang bringen kann. Das ist ökonomisch. Nur wenn dafür sozusagen eine Timebox abgelaufen ist, tut man gut daran, Alternativen in den Blick zu nehmen, das das wahre Ziel zu erreichen: den Geschäftstermin zu halten.

Wirklich helfendes Verhalten hätte also als erstes danach gefragt, was das eigentliche Ziel ist. "Warum brauchen Sie das Auto?" Und dann hätte man die Alternativen kurz betrachtet und abgewogen. Eine ist natürlich, zu versuchen, den Motor wieder in Gang zu bringen.

Aber zurück zu deinem Code:

Da muss die erste Erkenntnis sein: das Problem ist eben nicht, wie du das DIP anwenden kannst. Das Problem ist, zunächst mal herauszufinden, was denn der Code soll. Worum geht es? Was passiert idealerweise aus Sicht des Anwenders? Das hat nichts mit Klassen oder DIP zu tun.

Im zweiten Schritt geht es um die Abwägung der Alternativen. Dass es vielleicht ganz ohne Code ginge, lasse ich mal außen vor ;-) Soviel nehme ich als Kontext und gesetzt an.

Abwägung der Alternativen bedeutet für mich, dass ich eine Idee davon entwickle, wie der Code eigentlich aussehen sollte für die gewünschte Funktionalität. Er sieht irgendwie aus, ist halt Legacy Code. Aber wie sollte er aussehen? Auch das hat noch immer nichts mit DIP zu tun.

DIP ist ein technisches Mittel, um einen Entwurf, in dem Funktionalität auf Klassen verteilt ist, zu implementieren.

Dann erst kommt für mich die Frage, wie denn der Entwurf zu implementieren wäre. Ob dabei DIP zum Einsatz käme, weiß ich nicht. Und ob dabei Interfaces zum Einsatz kämen, weiß ich auch nicht. Ich habe ja grad keine Problemidee und schon gar keinen Lösungsansatz (Entwurf).

Bottom line: Die CCD Prinzipien sind nur bescheidene Mittel. Keines muss eingesetzt werden. Welches von den SOLID Prinzipien gut angewandt ist, entscheidet der Kontext.

Wenn du etwas über DIP schreiben willst, dann kannst du entweder "die Mechanik" zeigen. Das wird meistens getan. Da sieht man, wie es funktioniert. Das Pattern. In einem Minikontext, wo es irgendwie plausibel ist.

Oder du hast wirklich, wirklich einen Kontext im Sinne eines Entwurfs und einer Übersetzung des Entwurfs, wo es Sinn macht. Wo sich aus dem Entwurf und der davor liegenden Problemstellung ergibt, dass DIP ein wichtiges Mittel zur Erreichung irgendeines explizit zu benennenden Zieles ist.

Ersteres willst du nicht, Leba - glaub ich zumindest.

Für das Zweite fehlen Aufgabenstellung und Entwurf - zumindest bei deinem Posting hier. Aber das ist aus meiner Sicht der einzig  wertvolle Ansatz jenseits des Trivialen. Es geht ja um eine Seminararbeit, bei der du eine Transferleistung erbringen willst.

Versuch es doch nochmal. Liefere uns eine Problemstellung, von der du meinst, dass sie hinter dem Legacy Code steht. Schreibe eine kleine User Story. Da sollte beschrieben sein, wie ein Anwender mit dem Programm umgehen will. Was will der erreichen? Kann ja weiterhin eine Konsolenanwendung sein.

Und dann überlegen wir zusammen, wie ein "anatomisch gesunder" Entwurf dafür aussehen sollte.

Und dann überlegen wir, wie dafür Code aussehen sollte.

Und dann überlegen wir, wie der Legacy Code in welchen Schritten möglichst nah an das Ideal herangeführt werden kann.

Vielleicht kommen dabei ja auch SOLID-Prinzipien zum Einsatz :-)

Ich glaube ganz fest, dass wir nur so aus dem typischen reflexhaften Verhalten irgendeines Tooleinsatzes herauskommen. Nur so werden wir den CCD Bausteinen gerecht. Wir müssen sie als immer desponible Mittel sehen.

[Collin Rogowski]

Hmmmm. Hat nicht die kontextfreie Beschäftigung mit Mechaniken/Techniken auch einen Sinn? Nämlich eben genau diese Mechaniken/Techniken zu lernen / zu üben? Denn nur dann kann ich sie doch in "echten Situationen" -- also solchen mit Kontext -- sinnvoll einsetzen, bzw. bewusst entscheiden sie nicht einzusetzen (weil es in dem Kontext eben vielleicht bessere Lösungen gibt).

cr

--
Sie haben diese Nachricht erhalten, weil Sie der Google Groups-Gruppe Clean Code Developer beigetreten sind.
Besuchen Sie https://groups.google.com/d/msg/clean-code-developer/-/a9IEeiOm0EcJ, um diese Diskussion im Web anzuzeigen.
Wenn Sie Nachrichten in dieser Gruppe posten möchten, senden Sie eine E-Mail an clean-code...@googlegroups.com.
Wenn Sie aus dieser Gruppe austreten möchten, senden Sie eine E-Mail an clean-code-devel...@googlegroups.com.
Besuchen Sie die Gruppe unter http://groups.google.com/group/clean-code-developer?hl=de, um weitere Optionen zu erhalten.

[Leba]

Meine ursprüngliche Idee war zuerst die SOLID Principles kurz zu erläutern anhand eines einfachen, konstruierten Beispiels, eben um die "Mechanik" zu zeigen. Dabei ist unter anderem der oben beschriebene Code entstanden.

Im zweiten Teil der Arbeit wollte ich dann tatsächlichen Legacy Code aus einer produktiven Software nehmen und diesen bzgl. SP untersuchen.

Ich habe aber festgestellt, dass das völlig den Rahmen sprengt (Seminararbeit sollte 15 - 20 Seiten Umfang haben, und der erste Teil ist hat bis jetzt schon über 20 Seiten) und bin noch nicht so ganz sicher, wie ich jetzt weitermache.

Wie auch immer, bzgl. des Beispiels: Es muss nicht unbedingt eine Konsolenanwendung sein. Ich wollte eigentlich nur zeigen, dass es ein High Level Modul gibt, welches die Instanzen erzeugt und die Dependencies hereinreicht. Die Main Methode ist ein Beispiel, es könnte aber auch ein Unit Test sein, oder eine Web Applikation.

Ich versuch mal was. User Story könnte sein:

Um die Personalbuchhaltung führen zu können, möchte ich in der Rolle des Personalbearbeiters für einen Mitarbeiter des Back Office und des Marketing entsprechend des individuellen Monatseinkommens die jährliche Gehaltsabrechnung ausgeben (z.B. auf der Konsole) lassen.

Ich denke, der Code erfüllt diese Anforderung.

Tatsächlich bin ich unsicher, wie ich eine Anforderung formulieren könnte, die das DIP fordert.

Aber das wäre schon schön zu haben, da ich ja das DIP gerne vorstellen möchte.

Auch wenn das ein bisschen verdreht ist, erst den Code und dann die Anforderung zu beschreiben...

Wenn Sie aus dieser Gruppe austreten möchten, senden Sie eine E-Mail an clean-code-developer+unsub...@googlegroups.com.

[Ralf Westphal]

Klar, man muss auch mal nur Handgriffe üben. Dafür halte ich das beschriebene Beispiel aber für zu groß. Und wie schwierig ist es, eine Abhängigkeit zu injizieren?

Außerdem sind unsere Werkzeuge komplizierter als ein Hammer. Bei ihnen schwingt ganz schnell Paradigma, "Lebenseinstellung", Dogma mit. Sie sind raumgreifend - und daher, so meine ich, sehr kontextspezifisch. Viel kontextspezifischer als Hammer, Bohrer und Messer.

Bei einem Hammer bekomme ich auch viel, viel schneller Feedback, ob ich ihn erstens richtig benutze und zweitens ob sein Einsatz überhaupt angezeigt ist. Das kann man sehen, nein, im wahrsten Sinne des Wortes begreifen.

Bei einem Prinzip hingegen oder auch einem konkreten Tool, das für ein Paradigma steht, da kommt das Feedback nicht so schnell. Deshalb muss man nicht nur Handgriffe lernen, sondern immer auch den Kontext gleich dazu sehen. Sonst wird das Mittel eben schnell zum Zweck.

Am Dienstag, 15. Januar 2013 09:13:10 UTC+1 schrieb Collin Rogowski:

[Ralf Westphal]

DIP ist ein Mittel, um Code einer bestimmten Struktur einfach zu implementieren.

Die Struktur von Code ergibt sich aus dem Entwurf (sowie dahinter stehenden Prinzipien und Paradigmen.)

Der Entwurf ergibt sich aus Anforderungen.

Insofern weiß ich nicht, wie überhaupt jemand auf den Einsatz von DIP kommen kann, ohne Anforderungen zu haben. Dasselbe gilt für die TPL oder EF oder oder oder.

Wenn du nicht von den Anforderungen ausgehst, wird der Einsatz von DIP entweder unverständlich oder zum Selbstzweck. Wenn er zum Selbstzweck wird, dann kannst du ganz abstrakte Bezeichner wählen wie

interface IService {

  string F(int x);

}

class ServiceNehmer {

  IService _s;

  public ServiceNehmer(IService s) { _s = s; }

  public void TueEtwas(...)

  {

    ...

    var y = _s.F(...);

    ...

  }

}

class KonkreterService : IService

{

  public string F(int x) {

    ...

  }

}

...

var s = new KonkreterService();

var sn = new ServiceNehmer(s);

s.TueEtwas(...);

Fertig ist die Laube. Dazu eine kleine Grafik, wie die Abhängigkeiten laufen. Und ein kleiner Text, warum man sowas machen will. Das macht dann für das Prinzip 2 Seiten in der Seminararbeit. 18 to go ;-)

Deine Eigenleistung besteht darin, dass du die Literatur zum DIP etwas anders darstellst bzw. zusammenfasst. Dein Code, deine Grafik, dein Text.

Sobald du aber mit einer Problemdomäne ankommst - und die impliziert dein Beispiel - musst du einen Entwurf und eben auch Anforderungen haben. Aber in umgekehrter Reihenfolge. Und dann stellt sich die Frage, wie der Entwurf aussieht und ob dessen Übersetzung eine Dependency Injection fordert.

Dass eine DI angezeigt ist, ist ja nicht so schwer zu erreichen. Sogar leichter als in deinem Beispiel. Aber ich denke eben, dass die wirklich plausibel sein sollte.

Bei deiner User Story ist allerdings immer noch viel ungesagt. Da verstehe ich zum Beispiel noch gar nicht, warum es überhaupt die Klassen geben soll, die du in deinem Code hast :-) Mein Gefühl ist, dass nicht nur das DIP hier reingezwungen wird, sondern auch eine unglückliche Form der Objektorientierung. Tut mir leid, dass ich das so hart sagen muss. Aber genau aus solchen Beispielen erwachsen am Ende Programme, die eben nicht mehr wartbar sind. Die wahre Refaktorisierung deines Beispielcodes bestünde wahrscheinlich darin, mit diesen ganzen Klassen aufzuräumen, statt eine Abhängigkeit zu injizieren. Also Ernährungsumstellung statt Fett absaugen ;-)

Aber auch das ist nur eine Vermutung. Ich sehe ja leider immer noch keine nicht-trivialen Anforderungen, die solchen nicht-trivialen Strukturen rechtfertigen würden. Also habe ich auch noch keine Grundlage für einen Entwurf. Also kann ich eigentlich über Klassen und DIP gar nichts sagen.

[Leba]

Also ich habe mir jetzt nun mal das Thema Solid Principles ausgesucht, nicht zuletzt da ich sie selber erstmal richtig verstehen und lernen möchte.

Deswegen soll es auch um SP gehen und nicht um Use Case getriebene Entwicklung (was nicht heißt, dass ich das komplett vernachlässigen will).

Dazu kommt, dass ich jemand bin, der Konzepte gut versteht, wenn ich sie konkret anwenden kann. In diesem Fall bedeutet das, den Code zu schreiben und das Programm auszuführen. Ehrlich gesagt finde ich die abstrakte Beschreibung der Principles viel langweiliger.

Deswegen finde ich es auch reizvoll, die SP anhand einer kompletten Beispiel Applikation zu erläutern. Schließlich wird das in den meisten Texten im Netz und in den entsprechenden Büchern auch so gemacht. Allerdings verwenden die meisten für die jeweiligen Prinzipien unzusammenhängende Beispiele.

Ich glaube, dass ich doch einigermaßen verstanden habe, wann und warum man die SP verwendet. Vielleicht ist eine Thread wie dieser einfach nicht geeignet um das komplette Bild zu zeichnen. Ich habe versucht ein kleines Problem aus dem Kontext zu isolieren und merke, dass mich die Antworten in eine andere Richtung drängen, als ich gehofft habe. Das soll aber nicht heißen, dass ich die Antworten nicht als wertvoll betrachte.

Ich habe leider nicht mehr so viel Zeit, um die Arbeit komplett umzustrukturieren. Möglicherweise wäre das aber trotzdem angebracht? Hierzu würde ich natürlich auch gerne mal ein Feedback bekommen, aber dazu müsste ich die komplette Arbeit zur Verfügung stellen. Wenn jemand bereit wäre, sie zu lesen, würde ich mich natürlich sehr freuen!

Nochmal zu dem Beispiel:

Kann man nicht sagen, wir haben den o.g. Code, der vielleicht schon irgendwo in Produktion ist.

Es gibt eine Abteilung, die den MarketingAssistantCalculator verwendet. Von einer anderen Abteilung bekommen wir jetzt die Anforderung einen neuen MarketingAssistantCalculator zu implementieren, da sich nur für diese spezielle Abteilung die Berechnung des Einkommens eines MarketingAssistant geändert hat. Dies fordert doch das DIP? Ansonsten müssten wir die Klasse MarketingAssistant verändern und damit das OCP verletzen, oder?

vg Leba

[Ralf Westphal]

Ich verstehe, dass du keine Lust hast, deine Arbeit umzuschmeißen.

Du könntest sie mir schicken, ich würd sie auch lesen (sind ja nicht soviele Seiten) - aber ob ich dich mit meinem Feedback glücklich machen würde? ;-) Das ist ja schon hier verstörend für dich :-)

Nochmal zu deinem Beispiel: Ich finde das immer noch aus mehreren Gründen unschön. Erstens: Da passiert nichts. Selbst deine refaktorisierte Version tut nichts nach Aufruf. Wenn du schon den Anspruch an mehr als abstraktes Beispiel hast, dann mach doch auch eine Anwendung, die echt interaktiv ist.

Zweitens: Das ist wirklich übler Legacy Code. Du hast dir bestimmt das Hirn verwunden, dir sowas auszudenken. Denn das ist natürlich totaler Mist, Employee mit PrintAnnualPayment() auszustatten. Das ist schonmal SRP und SoC nicht beachtet. Und deshalb finde ich das ganze Rumgehüsere mit DIP auch so überflüssig. Es stimmt viel Grundsätzlicheres nicht. Da würde DIP kaschieren, wie ein schöner Strumpf das Raucherbein.

Deshalb bin ich wieder bei meinem Punkt: Du kannst sagen, "Solchen Legacy Code habe ich vorgefunden. Denn will ich refaktorisieren. Die neue Anforderung ist X." (sobald der denn auch etwas tut).

Dann machst du einen Entwurf, wie es richtig aussehen sollte. Und dann refaktorisierst du in Schritten zu einer Implementation, die den Entwurf widerspiegelt (Blauer Grad!).

Wenn dabei eine DI vorkommt, kannst du die hervorheben. Vielleicht kommt aber auch noch was anderes aus den SP vor.

[Josef Scherer]

Hallo zusammen,

Wäre toll wenn andere, aus ihrer Praxis ein solches Beispiel zum refactorn beitragen könnten.

Bin gespannt!

Josef

Von meinem iPhone gesendet

Besuchen Sie https://groups.google.com/d/msg/clean-code-developer/-/VRz7E8cKoDcJ, um diese Diskussion im Web anzuzeigen.

Wenn Sie Nachrichten in dieser Gruppe posten möchten, senden Sie eine E-Mail an clean-code...@googlegroups.com.

Wenn Sie aus dieser Gruppe austreten möchten, senden Sie eine E-Mail an clean-code-devel...@googlegroups.com.

[Leba]

Ja, ich fände es auch cool, wenn man mal eine Live-Refactoring-Session machen würde.

Jemand stellt Legacy Code zur Verfügung und man refactored gemeinsam und diskutiert darüber.

Leba