Clean Code Development: Umsetzung IOSP

  • Allgemein

Es gibt 13 Antworten in diesem Thema. Der letzte Beitrag () ist von ErfinderDesRades.

    Clean Code Development: Umsetzung IOSP

    Hallo zusammen.

    Meine Projekte mithilfe des Leitwerks der CCD zu bearbeiten, scheint wohl noch einiges an Zeit zu verschlingen. An einigen Stellen (hier IOSP) stockt es immer wieder, vielleicht könnt Ihr mir weiterhelfen. Das ganze ist m.E. sprachübergreifend, könnte genauso in C++, C# oder sonstewas auftauchen:

    1. Problem: Ich habe eine Funktion, die zu Beginn eines Kassenbuch-WinForms-Programms abläuft:

    VB.NET-Quellcode

    1. Private Sub SetupFormular()
    2. DtpSelectedDate.Value = My.Settings.LastUsedDate 'DTP = DateTimePicker
    3. RtxtAmountInCents.Enabled = DataSetHolder.TdsCashBook.Account.Count > 0 'Rtxt = spezielle TextBox
    4. CbxAccount.Enabled = DataSetHolder.TdsCashBook.Account.Count > 1 'Cbx = ComboBox
    5. If DataSetHolder.TdsCashBook.CashBookEntry.Count > 0 Then
    6. DtpSelectedDate.Value = DataSetHolder.TdsCashBook.CashBookEntry.Last.BookingDate
    7. RestrictDateInputToCurrentMonth()
    8. End If
    9. CbxBookingText.Text = Nothing
    10. End Sub

    Problematisch wird es für mich in Zeile 7. Der Code sollte ja schon klarmachen, worum es geht: Ich setze einige CE-Werte fest, will aber in jener 7. Zeile eine Funktion aufrufen, die mein DateTimePicker-CE deaktiviert und wertemäßig einschränkt, wenn bereits einige Einträge im laufenden Monat im Kassenbuch stehen. Lirum larum. IOSP ist verletzt. Was wär zu tun? Ich könnte die If-Geschichte in die Funktion rüberschieben und die Funktion selber nach dem Aufruf von SetupFormular() aufrufen. Der Punkt ist, dass ich das vermeiden wollte, da jene Funktion an anderen Stellen auch aufgerufen wird, die genannte Einschränkung aus Zeile 5 aber nur am Anfang relevant ist. Sei's drum, warum. Meine Frage an Euch: Was wären die Alternativen?

    2. Eine Funktion X ruft nacheinander die Funktionen A, B, C, D, E, F auf.

    VB.NET-Quellcode

    1. Private Sub X()
    2. A()
    3. B()
    4. C()
    5. D()
    6. E()
    7. F()
    8. End Sub

    In A steht die ein oder andere Abbruchbedingung, die dazu führen soll, dass die anderen Funktionen nicht abgearbeitet werden sollen.

    VB.NET-Quellcode

    1. Private Sub A()
    2. If SystemStatusIsChaotic Then
    3. CancelAnyFurtherActionAndQuitMyHolyProgramBeforeMyComputerExplodes = True
    4. Exit Sub
    5. End If
    6. '...

    IOSP würde m.E. bedeuten, dass diese Bedingung in X nicht als IF-Statement o.ä. auftauchen darf:

    VB.NET-Quellcode

    1. Private Sub X()
    2. A()
    3. If Not CancelAnyFurtherActionAndQuitMyHolyProgramBeforeMyComputerExplodes Then
    4. B()
    5. C()
    6. D()
    7. E()
    8. F()
    9. End If
    10. End Sub

    Aber wie macht man es, ohne jeweils innerhalb von B bis F erstmal prüfen zu müssen, ob die Abbruchbedingung aus A zutrifft. Fände ich nämlich nervtötend redundant. Vor allem, wenn in den Funktionen dann weitere Abbruchbedingungen dazukämen. Sollte man das Ganze in X in einen Try-Catch-Block packen und statt mit Boolean-Variablen eben mit Exceptions hantieren? Ist das dann noch IOSP-compliant? Oder B-F in eine eigene Funktion machen und diese mit einem If-Statement versehen? Letzteres halte ich wiederum für eine Verletzung von IOSP und SLA. Vorschläge?

    btw: Mir ist klar, dass CCD kein Gesetzeswerk, sondern eine Richtlinie ist. Aber die muss doch auch umsetzbar sein. Ich bin wohl aber noch nicht soweit, um da einen guten Weg zu erkennen.
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.

    Dieser Beitrag wurde bereits 2 mal editiert, zuletzt von „VaporiZed“ ()

    @VaporiZed Warum kein If? Kommt das wieder aus diesem Aprilscherz-Thread?
    So haste pro Funktion auch nur einen Einzeiler:

    VB.NET-Quellcode

    1. Private Function Test() As Boolean
    2. Dim flag = True
    3. If flag Then flag = a()
    4. If flag Then flag = b()
    5. If flag Then flag = c()
    6. If flag Then flag = d()
    7. Return flag
    8. End Function
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!
    Nö, kein Aprilscherz. Wär n bisken spät.
    Die ganzen If's wären ja wieder Kontrollstrukturen, hier vermischt mit Funktionsaufrufen a(), b(), ..., was ja genau IOSP widerspricht. Oder hab ich da n Denkfehler?
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.

    VB.NET-Quellcode

    1. Return a() AndAlso b() AndAlso ...


    Falls die damit tatsächlich das verbieten würden wären sie schon sehr dämlich. Aber da ist die Frage was deine Methoden machen, ob die sinnvoll sind. Wenn die Funktionen getrennt und in beliebiger Reihenfolge aufgerufen werden können, dann ist es egal. Wenn es nur private Methoden sind, dann ist es nur zur besseren Übersicht und passt somit auch. Wenn man sie jedoch nicht beliebig aufrufen darf aber kann, dann bist du es mMn falsch angegangen(Ausnahmen bestätigen die Regel->Connect :D)
    Ich wollte auch mal ne total überflüssige Signatur:
    ---Leer---
    @jvbsl:
    Hier Beispiel für eine IOSP-Prozedur in meinem Programm. Es schließt das momentane Kassenbuch, speichert es ab und lädt ein bereits abgespeichertes:

    VB.NET-Quellcode

    1. Private Sub LoadFinalizedMonth()
    2. FinalizeCurrentMonth()
    3. MakeFinalizedCashBookFileToCurrentCashBookFile() 'sollte in dieser Funktion keine sinnvolle Datei ausgewählt werden, darf der Rest nicht ausgeführt werden, aber eben IOSP-gerecht
    4. LinkDataSetToSharedClass(TdsCashBook)
    5. LoadDataFrom(RootDataFile, CashBookDataFile)
    6. SetupFormular()
    7. SetAccountFilter()
    8. End Sub

    Reihenfolge ist zwingend so notwendig. Ich habe es zwar so programmiert, dass dann, wenn keine alte Kassenbuchdatei gewählt wurde, das Kassenbuch einfach leer bleibt und der Rest trotzdem abgearbeitet werden kann. Aber ist ja auch nur ein Beispiel. Sollten Daten zwingend aus einer Datei notwendig sein, muss ja ein Abbruch erfolgen können, wenn man die Dateiauswahl abbricht. Und es geht mir eben darum, wie man es strukturell IOSP-konform hinbekommt. That's the topic. Mit If und Exceptions ist mir schon klar. Sowas zu basteln ist keine Hürde (für mich). Aber es geht ja hier nicht einfach darum, eine simple wenn-Abbruchbedingung-auftritt-dann-überspring-den-Rest-Geschichte zu basteln. Wär ja langweilig. Es geht darum, entweder nur Assembly-eigene Prozeduren aufzurufen oder nur Kontrollstrukturen abzuarbeiten. Das ist meines Erachtens IOSP. Das Vermischen ist zu vermeiden.
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.

    Dieser Beitrag wurde bereits 3 mal editiert, zuletzt von „VaporiZed“ ()

    VB.NET-Quellcode

    1. Private Sub LoadFinalizedMonth()
    2. FinalizeCurrentMonth()
    3. if not MakeFinalizedCashBookFileToCurrentCashBookFile() then Return
    4. LinkDataSetToSharedClass(TdsCashBook)
    5. LoadDataFrom(RootDataFile, CashBookDataFile)
    6. SetupFormular()
    7. SetAccountFilter()
    8. End Sub
    Keine Ahnung ob das IOSP ist - ich finds jdfs. gut lesbar und der Verschachtelung-Faktor ist denkbar niedrig.
    Ich empfehle immer gerne, sich nicht verrückt machen zu lassen von Pattern.
    Wenn sie helfen ist gut, wenns in Krampf ausartet, artets in Krampf aus.
    Ich glaub ja, Jesus war ein Programmierer - der sagte doch: "Das Gesetz ist für den Menschen da, nicht der Mensch für das Gesetz".

    VaporiZed schrieb:

    was ja genau IOSP widerspricht
    Dann erwarte ich von IOSP ein Beispiel wie es sein soll, sowie ein Gegenbeispiel, wie es nicht sein soll.
    Alles andere wäre dummes Gelaber.
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!
    Ich versuch mal in der kommenden Zeit ein paar Beispiele zu recherchieren.
    Mir kam folgendes in den Sinn. Hatte ich zwar schon in ähnlicher Form angedeutet, aber so ginge es eben alternativ. Ob das jetzt IOSP-konform ist, weiß ich zwar auch nicht, aber wär jetzt das nächste, was mir eben einfällt. (Wobei alles bisher Besprochene bisher nur auf Problem 2 eingeht, aber nur keine Hektik):

    VB.NET-Quellcode

    1. Private Sub TryToLoadFinalizedMonth()
    2. Try
    3. LoadFinalizedMonth()
    4. Catch BlaBlaBla As BlaBlaBlaException 'hier alle auftretenden selbstgeworfenen (!) Exceptions aus A-F abfangen und so die Aufrufabfolge unterbrechen
    5. End Try
    6. End Sub

    => LoadFinalizedMonth bliebe operation-clean; keine Abbruchvariablen, die man auswerten müsste; Abbruch der Reihenfolge ohne Probleme zu jedem Zeitpunkt möglich

    Mir ist klar, dass das genannte keine gedankliche Glanzleistung ist und dass da jeder hier zurecht sagen würde: »Ja klar, so hätte ich es auch gemacht, hätte man mich gefragt.« und dass es das ganze insofern komplizierter macht, weil dann plötzlich im Programm solche TryToMakeSomeBullshit-Prozeduren wie Pilze aus dem Boden schießen, aber kann ja sein, dass irgendjemand solch eine Prinzipienfrage nochmal vorhat zu stellen. Ggf. ergeben sich ja hier auch noch der ein oder andere bessere Designansatzpunkt für den ein oder anderen Programmierneuling.
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.

    Dieser Beitrag wurde bereits 1 mal editiert, zuletzt von „VaporiZed“ ()

    Jo - mit dem Gedanken, TryCatch dafür zu missbrauchen, habich auch gespielt.
    Ist aber gängiges Dogma, dass Exceptions nicht für Programmfluss-Steuerung zu missbrauchen sind.
    Aber du weisst ja, was ich von Dogmen halte ;)

    Auch kann man mal einen Blick auf die neueste Threading-Technologie werfen, nämlich das System.Threading.Task-Modell:
    Dort ist vorgesehen, dass wenn der User einen Thread canceln will, dass dann das CancellationToken eine OperationCanceledException wirft.
    Ein klarer Bruch des erwähnten Dogmas, und zwar von MS themself!

    Aber schau nochmal mein post#6-Code an: IOSP hin oder her - ich find den leserlicher als sone doch etwas eigenwillige TryCatch-Konstruktion.



    Zu deim ersten Problem: Was ist ein CE?
    Egal - was immer es ist, vlt. ist folgender Hint in die richtige Richtung:

    VB.NET-Quellcode

    1. Private Sub SetupFormular()
    2. RtxtAmountInCents.Enabled = DataSetHolder.TdsCashBook.Account.Count > 0 'Rtxt = spezielle TextBox
    3. CbxAccount.Enabled = DataSetHolder.TdsCashBook.Account.Count > 1 'Cbx = ComboBox
    4. DtpSelectedDate.Value = GetStartupSelectedDate()
    5. CbxBookingText.Text = Nothing
    6. End Sub

    Und das Brimborium mittm Datum verlegst du in GetStartupSelectedDate() hinein
    Ein Beispiel für IOSP
    @ErfinderDesRades: CE - hehe. Siehe meine Signatur: control element.
    Die Funktion selber ist derzeit:

    VB.NET-Quellcode

    1. Private Sub RestrictDateInputToCurrentMonth()
    2. DtpSelectedDate.Enabled = False
    3. DtpSelectedDate.MinDate = Date.Parse(String.Join(".", "01", DtpSelectedDate.Value.Month, DtpSelectedDate.Value.Year))
    4. DtpSelectedDate.MaxDate = Date.Parse(String.Join(".", "01", DtpSelectedDate.Value.Month, DtpSelectedDate.Value.Year)).AddMonths(1).AddDays(-1)
    5. End Sub

    Es werden also Zugang und Min-/Max-Werte eingeschränkt, sodass aktuelle Kassenbucheinträge nur im laufenden Monat vorgenommen werden dürfen. Dieser Aufruf erfolgt pauschal nach jeder Buchung. In der Prozedur SetupFormular() wird eben erst geschaut, ob was im aktuellen KaBu schon drinsteht. Wenn ja, kommen die Einschränkungen, sonst eben nicht und man kann sich den Monat raussuchen. Die Funktionalität ist erstmal nur dafür da, weil ich alte Papierbestände digitalisiere. Für ein laufendes Kassenbuch ist das natürlich unnötig.
    Klar, ich könnte die If-Geschichte aus Post#1 Codezeile 5 einfach hier einbauen, aber es gehört zum Startprozess, nicht zum Buchen. Gut, die Einschränkungen jener Funktion gehören eigentlich auch nicht zum Buchen. Hm. Vielleicht war es ein schlecht gewähltes Beispiel.
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.

    Dieser Beitrag wurde bereits 1 mal editiert, zuletzt von „VaporiZed“ ()

    VaporiZed schrieb:

    Siehe meine Signatur: control element.
    Ich sehe deine Signatur nicht - Signaturen blende ich aus.



    Jo, das Beispiel kommt mir sogar bekannt vor.
    Aber ma ehrlich: Was ist von einem Coder zu halten, der für jede Methode eine eigene Klasse aufsetzt?
    Und findest du den Code nach Refaktorierung wirklich spürbar besser als vorher?
    Vorher warens 21 Zeilen, hinterher sinds 35.

    Hingegen den Abstrakta stimme ich zu:

    Quellcode

    1. a->b
    2. a->c
    3. a->d
    4. a->e

    ist besser als

    Quellcode

    1. a->b
    2. b->c
    3. c->d
    4. d->e
    Weil bei ersterem sieht man, wenn man a anguckt bereits alle beteiligten Akteure.
    Bei letzterem sieht man immer nur den nächsten Akteur, und muss sich von einem zum andern hangeln - definitiv schlechtere Übersicht!
    @VaporiZed Die Nicht-Testbarkeit da zu postulieren ist m.E. schwachsinnig, ich verweise da mal auf meine Signatur:
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Hier ist ein automatischer Test, der beliebige Anfangs- und Randbedingungen berücksichtigt, der bessere Weg.
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!
    Da ja immer mal wieder die Bitte kommt, angefangene Themen abzuschließen, wenn sie so ziemlich erledigt sind, schließ ich auch dieses Thema endlich ab, nachdem ich irgendwann mal u.a. auch gelesen habe, dass die Erfinder und Verteidiger des IOSP Einsehen haben und sich bewusst sind, dass For-Each-Schleifen und IFs hinnehmbar sind, solange die Integrations-Subs leicht zu testen sind, wenn keine oder offensichtlich einfach zu testende Abhängigkeiten dort auftauchen. For-Each, solange wirklich alle Elemente in der Integrations-Sub verarbeitet werden (Ausnahmen werden dann in der Operationssub rausgefiltert) und bei IF, wenn es leicht verständliche Abbruchbedingungen zum Anfang sind, die jedoch nicht in der Integrationssub ausklamüsert werden, sondern in einer eigenen Function. Also sowas ist anscheinend als Integrationssub ok:

    VB.NET-Quellcode

    1. Private Sub ProcessAllFilesIn(FileList As List(Of IO.FileInfo))
    2. If Not FileListIsValid(FileList) Then Exit Sub
    3. For Each File In FileList
    4. ProcessFile(File)
    5. Next
    6. End Sub

    Und ProcessFile ist dann entweder wieder Integrationssub. Oder reine Operationssub, die dann aber keine weiteren Subs der Assembly aufrufen darf. Framework-Subs, DLL-Subs oder (m.E.) auch Funktionen aller Art sind aber ok.

    Zum Thema Try-Catch als Programmsteuerung (Post#8, #9): Das habe ich ausgetestet, kommt mir aber insofern falsch vor, weil es einen Missbrauch jener guten Fehlerverfolgungsfunktionalität darstellt. Ist ja doch hier nur ein GoTo-Ersatz. Und außerdem - wie einige selbst schon rausgefunden haben - ist das Werfen einer Exception sehr performancelastig.
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.
    Ich muss sagen, um IOSP habich mich nie gekümmert. Trotzdem denke ich, dass meine Codes nicht allzusehr dem widersprechen.
    Weil ich verfahre nach einer ganz anneren Heuristik, aber mit durchaus vergleichbarem Ergebnis:
    Wird eine Methode zu lang, etwa > 40 Zeilen, fange ich an, Einheiten auszulagern.
    Und was ich dann auslager, ist eiglich immer, was im IOSP "Operation" genannt wird :D .

    Ansonsten kann man die Auslagerei auch übertreiben. Bei einer Methode, die nur von einer Stelle aufgerufen wird, stelle ich die Existenzberechtigung in Frage.
    Weil der Code könnte ja ebensogut im (einzigen) Aufrufer stehen, und damit hätte man - bei kleinen Sachen - besser im Blick, was vorgeht.