Verstößt ein Konstruktor, der seine Argumente überprüft, gegen SRP?

Ich versuche, mich so weit wie möglich an das Prinzip der einheitlichen Verantwortung (Single Responsibility Principle, SRP) zu halten, und habe mich an ein bestimmtes Muster (für die SRP-Methode) gewöhnt, das sich stark auf Delegierte stützt. Ich würde gerne wissen, ob dieser Ansatz richtig ist oder ob es ernsthafte Probleme damit gibt.

Um beispielsweise die Eingabe in einen Konstruktor zu überprüfen, könnte ich die folgende Methode einführen (die StreamEingabe ist zufällig, kann beliebig sein).

private void CheckInput(Stream stream)
{
    if(stream == null)
    {
        throw new ArgumentNullException();
    }

    if(!stream.CanWrite)
    {
        throw new ArgumentException();
    }
}

Diese Methode macht (wohl) mehr als eine Sache

• Überprüfen Sie die Eingaben
• Wirf verschiedene Ausnahmen

Zur Einhaltung der SRP habe ich daher die Logik auf geändert

private void CheckInput(Stream stream, 
                        params (Predicate<Stream> predicate, Action action)[] inputCheckers)
{
    foreach(var inputChecker in inputCheckers)
    {
        if(inputChecker.predicate(stream))
        {
            inputChecker.action();
        }
    }
}

Was angeblich nur eins tut (oder?): Überprüfen Sie die Eingabe. Für die eigentliche Überprüfung der Eingaben und das Auslösen der Ausnahmen habe ich Methoden wie eingeführt

bool StreamIsNull(Stream s)
{
    return s == null;
}

bool StreamIsReadonly(Stream s)
{
    return !s.CanWrite;
}

void Throw<TException>() where TException : Exception, new()
{
    throw new TException();
}

und kann CheckInputgerne anrufen

CheckInput(stream,
    (this.StreamIsNull, this.Throw<ArgumentNullException>),
    (this.StreamIsReadonly, this.Throw<ArgumentException>))

Ist das besser als die erste Option überhaupt, oder füge ich unnötige Komplexität hinzu? Gibt es eine Möglichkeit, dieses Muster noch zu verbessern, wenn es überhaupt machbar ist?

SRP ist vielleicht das am meisten missverstandene Software-Prinzip.

Eine Softwareanwendung wird aus Modulen erstellt, die aus Modulen erstellt werden, die aus ...

Im unteren Bereich enthält eine einzelne Funktion wie beispielsweise CheckInputnur ein kleines Stück Logik. Wenn Sie jedoch nach oben gehen, kapselt jedes nachfolgende Modul mehr und mehr Logik. Dies ist normal .

Bei SRP geht es nicht um eine einzelne atomare Aktion. Es geht darum, eine einzige Verantwortung zu haben, auch wenn diese mehrere Aktionen erfordert ... und letztendlich geht es um Wartung und Testbarkeit :

• es fördert die Einkapselung (Vermeidung von Gottesobjekten),
• es fördert die Trennung von Bedenken (Vermeidung von Änderungen durch die gesamte Codebasis),
• Die Testbarkeit wird verbessert, indem der Verantwortungsbereich eingeschränkt wird.

Die Tatsache, dass CheckInputmit zwei Prüfungen und zwei verschiedenen Ausnahmen implementiert wird , ist in gewissem Maße irrelevant .

CheckInputhat eine enge Verantwortung: Sicherstellen, dass die Eingabe den Anforderungen entspricht. Ja, es gibt mehrere Anforderungen, aber dies bedeutet nicht, dass es mehrere Verantwortlichkeiten gibt. Ja, Sie könnten die Schecks aufteilen, aber wie würde das helfen? Irgendwann müssen die Schecks in irgendeiner Weise aufgelistet werden.

Lass uns vergleichen:

Constructor(Stream stream) {
    CheckInput(stream);
    // ...
}

gegen:

Constructor(Stream stream) {
    CheckInput(stream,
        (this.StreamIsNull, this.Throw<ArgumentNullException>),
        (this.StreamIsReadonly, this.Throw<ArgumentException>));
    // ...
}

Jetzt CheckInputmacht weniger ... aber sein Anrufer macht mehr!

Sie haben die Liste der Anforderungen von dem Ort CheckInput, an dem sie gekapselt sind, zu Constructordem Ort verschoben , an dem sie sichtbar sind.

Ist es eine gute Abwechslung? Es hängt davon ab, ob:

• Wenn CheckInputnur dort aufgerufen wird: Es ist umstritten, einerseits macht es die Anforderungen sichtbar, andererseits wirft es den Code durcheinander;
• Wenn CheckInputmehrmals mit denselben Anforderungen aufgerufen wird , verletzt dies DRY und Sie haben ein Kapselungsproblem.

Es ist wichtig zu wissen, dass eine einzelne Verantwortung viel Arbeit bedeuten kann . Das "Gehirn" eines selbstfahrenden Autos hat eine einzige Verantwortung:

Das Auto zum Ziel fahren.

Es liegt in einer Verantwortung, erfordert jedoch die Koordination zahlreicher Sensoren und Akteure, die Entscheidungsfindung und möglicherweise widersprüchliche Anforderungen ¹ ...

... es ist jedoch alles gekapselt. Dem Kunden ist das also egal.

¹ Sicherheit der Passagiere, Sicherheit anderer, Einhaltung von Vorschriften, ...

Zitat von Onkel Bob zur SRP ( https://8thlight.com/blog/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html ):

Das Single Responsibility Principle (SRP) besagt, dass jedes Softwaremodul nur einen Grund für eine Änderung haben sollte.

... Bei diesem Prinzip geht es um Menschen.

... Wenn Sie ein Softwaremodul schreiben, möchten Sie sicherstellen, dass Änderungen nur von einer einzelnen Person oder vielmehr von einer einzelnen eng gekoppelten Gruppe von Personen stammen können, die eine einzelne eng definierte Geschäftsfunktion darstellen.

... Aus diesem Grund setzen wir SQL nicht in JSPs ein. Aus diesem Grund generieren wir kein HTML in den Modulen, die Ergebnisse berechnen. Aus diesem Grund sollten Geschäftsregeln das Datenbankschema nicht kennen. Aus diesem Grund trennen wir Bedenken.

Er erklärt, dass Softwaremodule auf die Sorgen bestimmter Stakeholder eingehen müssen. Beantworten Sie daher Ihre Frage:

Ist das besser als die erste Option überhaupt, oder füge ich unnötige Komplexität hinzu? Gibt es eine Möglichkeit, dieses Muster noch zu verbessern, wenn es überhaupt machbar ist?

IMO, Sie schauen nur auf eine Methode, wenn Sie auf einer höheren Ebene (in diesem Fall Klassenebene) suchen sollten. Vielleicht sollten wir einen Blick darauf werfen, was Ihre Klasse gerade tut (und dies erfordert weitere Erläuterungen zu Ihrem Szenario). Im Moment macht Ihre Klasse immer noch dasselbe. Wenn beispielsweise morgen eine Änderungsanforderung für eine Validierung vorliegt (z. B. "Jetzt kann der Stream null sein"), müssen Sie immer noch zu dieser Klasse gehen und die Inhalte darin ändern.

Nein, diese Änderung wird vom SRP nicht mitgeteilt.

Fragen Sie sich, warum in Ihrem Checker keine Überprüfung auf "das übergebene Objekt ist ein Stream" erfolgt . Die Antwort liegt auf der Hand: Die Sprache verhindert, dass der Aufrufer ein Programm kompiliert , das einen Nicht-Stream übergibt.

Das Typensystem von C # ist nicht ausreichend, um Ihre Anforderungen zu erfüllen. Ihre Prüfungen implementieren die Durchsetzung von Invarianten, die heute nicht im Typensystem ausgedrückt werden können . Wenn es eine Möglichkeit gäbe, zu sagen, dass die Methode einen schreibbaren Datenstrom ohne Nullwert akzeptiert, hätten Sie dies geschrieben, dies ist jedoch nicht der Fall. Sie haben also das nächstbeste getan: Sie haben die Typeinschränkung zur Laufzeit erzwungen. Hoffentlich haben Sie es auch dokumentiert, damit Entwickler, die Ihre Methode verwenden, nicht dagegen verstoßen müssen, ihre Testfälle nicht bestehen und das Problem dann beheben können.

Das Hinzufügen von Typen zu einer Methode verstößt nicht gegen das Prinzip der Einzelverantwortung. Die Methode erzwingt auch nicht ihre Vorbedingungen oder behauptet ihre Nachbedingungen.

Nicht alle Verantwortlichkeiten sind gleich.

Hier sind zwei Schubladen. Sie haben beide eine Verantwortung. Sie haben alle Namen, mit denen Sie wissen, was in sie gehört. Eines ist die Besteckschublade. Der andere ist die Müllschublade.

Was ist der Unterschied? Die Besteckschublade macht deutlich, was nicht dazugehört. Die Müllschublade nimmt jedoch alles auf, was passt. Das Herausnehmen der Löffel aus der Besteckschublade scheint sehr falsch. Trotzdem fällt es mir schwer, an etwas zu denken, das ich verpassen würde, wenn ich es aus der Müllschublade nehmen würde. Die Wahrheit ist, dass Sie behaupten können, dass alles eine einzige Verantwortung hat, aber welche hat Ihrer Meinung nach die konzentriertere einzige Verantwortung?

Ein Objekt mit einer einzigen Verantwortung bedeutet nicht, dass hier nur eines passieren kann. Verantwortlichkeiten können sich nisten. Aber diese Verschachtelungspflichten sollten Sinn machen, Sie nicht überraschen, wenn Sie sie hier finden, und Sie sollten sie vermissen, wenn sie weg sind.

Also, wenn Sie anbieten

CheckInput(Stream stream);

Ich mache mir keine Sorgen, dass sowohl Eingaben überprüft als auch Ausnahmen geworfen werden. Ich wäre besorgt, wenn es sowohl die Eingabe überprüfen als auch die Eingabe speichern würde. Das ist eine böse Überraschung. Eine, die ich nicht verpassen würde, wenn sie weg wäre.

Wenn Sie sich in Knoten binden und seltsamen Code schreiben, um sich an ein wichtiges Software-Prinzip zu halten, haben Sie das Prinzip normalerweise falsch verstanden (obwohl das Prinzip manchmal falsch ist). Wie die ausgezeichnete Antwort von Matthieu zeigt, hängt die gesamte Bedeutung von SRP von der Definition von "Verantwortung" ab.

Erfahrene Programmierer sehen diese Prinzipien und beziehen sie auf Erinnerungen an Code, den wir vermasselt haben. weniger erfahrene Programmierer sehen sie und haben möglicherweise überhaupt nichts mit ihnen zu tun. Es ist eine Abstraktion, die im Raum schwebt, alle grinsen und keine Katze. Also raten sie, und es geht normalerweise schlecht. Bevor Sie das Programmieren von Horse Sense entwickelt haben, ist der Unterschied zwischen seltsamem, überkompliziertem Code und normalem Code überhaupt nicht offensichtlich.

Dies ist kein religiöses Gebot, das Sie unabhängig von persönlichen Konsequenzen befolgen müssen. Es ist eher eine Faustregel, die ein Element der Programmierung des Pferdesinns formalisieren und Ihnen helfen soll, Ihren Code so einfach und klar wie möglich zu halten. Wenn es den gegenteiligen Effekt hat, suchen Sie zu Recht nach externen Eingaben.

Beim Programmieren kann man nicht viel falsches tun, als zu versuchen, die Bedeutung eines Bezeichners aus den ersten Prinzipien abzuleiten, indem man nur darauf starrt, und das gilt für Bezeichner, die über das Programmieren geschrieben werden, genauso wie für Bezeichner im tatsächlichen Code.

CheckInput-Rolle

Zuerst lassen Sie mich da draußen , das Offensichtliche gesagt, CheckInput ist eine Sache zu tun, auch wenn es verschiedene Aspekte überprüft. Letztendlich wird die Eingabe überprüft . Man könnte argumentieren, dass es keine Sache ist, wenn Sie mit aufgerufenen Methoden arbeiten DoSomething, aber ich denke, es ist sicher anzunehmen, dass das Überprüfen von Eingaben nicht zu vage ist.

Das Hinzufügen dieses Musters für Vergleichselemente kann hilfreich sein, wenn Sie nicht möchten, dass die Logik zum Überprüfen der Eingabe in Ihre Klasse eingefügt wird, dieses Muster jedoch eher ausführlich für das ist, was Sie erreichen möchten. Es könnte viel direkter sein, einfach eine Schnittstelle IStreamValidatormit einer einzelnen Methode zu übergeben, isValid(Stream)wenn Sie dies wünschen. Jede Klassenimplementierung IStreamValidatorkann Prädikate verwenden, wie StreamIsNulloder StreamIsReadonlywenn sie es wünschen, aber um auf den zentralen Punkt zurückzukommen, ist es eine ziemlich lächerliche Änderung, um das Prinzip der einheitlichen Verantwortung beizubehalten.

Gesundheitsüberprüfung

Ich bin der Meinung, dass wir alle einen "Sanity Check" durchführen dürfen, um sicherzustellen, dass Sie mindestens mit einem Stream arbeiten, der nicht null und beschreibbar ist, und dieser grundlegende Check macht Ihre Klasse nicht zu einem Validator für Streams. Wohlgemerkt, ausgefeiltere Schecks lassen Sie am besten außerhalb Ihrer Klasse, aber hier wird die Linie gezogen. Sobald Sie beginnen müssen, den Status Ihres Streams zu ändern, indem Sie ihn lesen oder Ressourcen für die Validierung verwenden, haben Sie damit begonnen, eine formale Validierung Ihres Streams durchzuführen. Dies sollte in eine eigene Klasse gezogen werden.

Fazit

Wenn Sie ein Muster anwenden, um einen Aspekt Ihrer Klasse besser zu organisieren, sollten Sie meiner Meinung nach in einer eigenen Klasse sein. Da ein Muster nicht passt, sollten Sie sich auch fragen, ob es überhaupt zu einer eigenen Klasse gehört. Ich bin der Meinung, dass das von Ihnen beschriebene Muster eine gute Idee ist, auch wenn Sie nicht glauben, dass sich die Validierung des Streams in Zukunft wahrscheinlich ändern wird zunächst trivial sein. Andernfalls müssen Sie Ihr Programm nicht beliebig komplexer gestalten. Nennen wir einen Spaten einen Spaten. Die Validierung ist eine Sache, aber die Überprüfung auf NULL-Eingaben ist keine Validierung, und daher können Sie sicher sein, dass Sie sie in Ihrer Klasse behalten, ohne das Prinzip der alleinigen Verantwortung zu verletzen.

Das Prinzip besagt ausdrücklich nicht, dass ein Stück Code "nur eine einzige Sache tun" soll.

"Verantwortung" in SRP sollte auf der Anforderungsstufe verstanden werden. Die Verantwortung von Code besteht darin, die Geschäftsanforderungen zu erfüllen. SRP wird verletzt, wenn ein Objekt mehr als eine unabhängige Geschäftsanforderung erfüllt . Unabhängig bedeutet dies, dass sich eine Anforderung ändern kann, während die andere Anforderung bestehen bleibt.

Ist es denkbar, dass eine neue Geschäftsanforderung eingeführt wird, die bedeutet, dass dieses bestimmte Objekt nicht auf Lesbarkeit überprüft werden sollte , während für eine andere Geschäftsanforderung das Objekt weiterhin auf Lesbarkeit überprüft werden muss? Nein, da in den Geschäftsanforderungen keine Implementierungsdetails auf dieser Ebene angegeben sind.

Ein aktuelles Beispiel für eine SRP-Verletzung wäre folgender Code:

var message = "Your package will arrive before " + DateTime.Now.AddDays(14);

Dieser Code ist sehr einfach, dennoch ist es denkbar, dass sich der Text unabhängig vom voraussichtlichen Liefertermin ändert, da diese von verschiedenen Teilen des Geschäfts entschieden werden.

Ich mag den Punkt aus der Antwort von @ EricLippert :

Fragen Sie sich, warum Ihr Checker nicht überprüft, ob es sich bei dem übergebenen Objekt um einen Stream handelt . Die Antwort liegt auf der Hand: Die Sprache verhindert, dass der Aufrufer ein Programm kompiliert , das einen Nicht-Stream übergibt.

Das Typensystem von C # ist nicht ausreichend, um Ihre Anforderungen zu erfüllen. Ihre Prüfungen implementieren die Durchsetzung von Invarianten, die heute nicht im Typensystem ausgedrückt werden können . Wenn es eine Möglichkeit gäbe, zu sagen, dass die Methode einen schreibbaren Datenstrom ohne Nullwert akzeptiert, hätten Sie dies geschrieben, dies ist jedoch nicht der Fall. Sie haben also das nächstbeste getan: Sie haben die Typeinschränkung zur Laufzeit erzwungen. Hoffentlich haben Sie es auch dokumentiert, damit Entwickler, die Ihre Methode verwenden, nicht dagegen verstoßen müssen, ihre Testfälle nicht bestehen und das Problem dann beheben können.

EricLippert hat Recht, dass dies ein Problem für das Typensystem ist. Und da Sie das Single-Responsibility-Prinzip (SRP) anwenden möchten, benötigen Sie grundsätzlich das Typensystem, um für diesen Job verantwortlich zu sein.

Es ist tatsächlich möglich, dies in C # zu sortieren. Wir können die Literalen nullzur Kompilierungszeit abfangen und die Nicht-Literalen nullzur Laufzeit abfangen . Das ist nicht so gut wie eine vollständige Überprüfung zur Kompilierungszeit, aber es ist eine strikte Verbesserung gegenüber dem Nie-Fangen zur Kompilierungszeit.

Weißt du, wie C # funktioniert Nullable<T>? Lass uns das umkehren und ein NonNullable<T>:

public struct NonNullable<T> where T : class
{
    public T Value { get; private set; }
    public NonNullable(T value)
    {
        if (value == null) { throw new NullArgumentException(); }
        this.Value = value;
    }
    //  Ease-of-use:
    public static implicit operator T(NonNullable<T> value) { return value.Value; }
    public static implicit operator NonNullable<T>(T value) { return new NonNullable<T>(value); }

    //  Hack-ish overloads that prevent null-literals from being implicitly converted into NonNullable<T>'s.
    public static implicit operator NonNullable<T>(Tuple<T> value) { return new NonNullable<T>(value.Item1); }
    public static implicit operator NonNullable<T>(Tuple<T, T> value) { return new NonNullable<T>(value.Item1); }
}

Nun, anstatt zu schreiben

public void Foo(Stream stream)
{
  if (stream == null) { throw new NullArgumentException(); }

  // ...method code...
}

, einfach schreiben:

public void Foo(NonNullable<Stream> stream)
{
  // ...method code...
}

Dann gibt es drei Anwendungsfälle:

1. Benutzer ruft Foo()mit einer Nicht-Null an Stream:

   Stream stream = new Stream();
   Foo(stream);

   Dies ist der gewünschte Anwendungsfall und funktioniert mit oder ohne NonNullable<>.

2. Benutzer ruft Foo()mit einer Null an Stream:

   Stream stream = null;
   Foo(stream);

   Dies ist ein Aufruffehler. Hiermit können Sie NonNullable<>den Benutzer darüber informieren, dass sie dies nicht tun sollten, dies kann sie jedoch nicht stoppen. In jedem Fall ergibt sich eine Laufzeit NullArgumentException.

3. Benutzer ruft Foo()mit null:

   Foo(null);

   nullwird nicht implizit in eine konvertiert NonNullable<>, sodass der Benutzer vor der Laufzeit einen Fehler in der IDE erhält . Dies delegiert die Nullprüfung an das Typsystem, so wie es die SRP empfehlen würde.

Sie können diese Methode erweitern, um auch andere Argumente geltend zu machen. Da Sie beispielsweise einen beschreibbaren Stream wünschen, können Sie einen definieren struct WriteableStream<T> where T:Stream, der sowohl für nullals auch stream.CanWritefür den Konstruktor prüft . Dies wäre immer noch eine Laufzeitprüfung, aber:

1. Es verziert den Typ mit dem WriteableStreamQualifier und signalisiert die Notwendigkeit für Anrufer.

2. Die Prüfung wird an einer einzelnen Stelle im Code ausgeführt, sodass Sie die Prüfung nicht throw InvalidArgumentExceptionjedes Mal wiederholen müssen.

3. Es entspricht besser dem SRP, indem die Typprüfungspflichten auf das Typsystem übertragen werden (wie von den allgemeinen Dekorateuren erweitert).

Ihr Ansatz ist derzeit prozedural. Sie brechen das StreamObjekt auseinander und validieren es von außen. Tun Sie das nicht - es bricht die Kapselung. Lassen Sie das Streamfür seine eigene Validierung verantwortlich sein. Wir können nicht versuchen, das SRP anzuwenden, bis wir einige Klassen haben, auf die wir es anwenden können.

Hier ist eine, Streamdie eine Aktion nur dann ausführt, wenn sie die Validierung besteht:

class Stream
{
    public void someAction()
    {
        if(!stream.canWrite)
        {
            throw new ArgumentException();
        }

        System.out.println("My action");
    }
}

Aber jetzt verletzen wir SRP! "Eine Klasse sollte nur einen Grund haben, sich zu ändern." Wir haben eine Mischung aus 1) Validierung und 2) tatsächlicher Logik. Wir haben zwei Gründe, die sich ändern müssen.

Wir können dies mit der Validierung von Dekorateuren lösen . Zuerst müssen wir unsere Streamin eine Schnittstelle konvertieren und diese als konkrete Klasse implementieren.

interface Stream
{
    void someAction();
}

class DefaultStream implements Stream
{
    @Override
    public void someAction()
    {
        System.out.println("My action");
    }
}

Wir können nun einen Decorator schreiben, der ein umschließt Stream, eine Validierung durchführt und sich auf die Streamfür die tatsächliche Logik der Aktion angegebene verschiebt.

class WritableStream implements Stream
{
    private final Stream stream;

    public WritableStream(final Stream stream)
    {
        this.stream = stream;
    }

    @Override
    public void someAction()
    {
        if(!stream.canWrite)
        {
            throw new ArgumentException();
        }
        stream.someAction();
    }
}

Wir können diese nun nach Belieben komponieren:

final Stream myStream = new WritableStream(
    new DefaultStream()
);

Möchten Sie eine zusätzliche Validierung? Fügen Sie einen weiteren Dekorateur hinzu.

Die Aufgabe einer Klasse ist es, eine Dienstleistung zu erbringen, die einem Vertrag entspricht . Eine Klasse hat immer einen Vertrag: eine Reihe von Anforderungen für die Verwendung und verspricht, dass sie über ihren Status und ihre Ergebnisse Auskunft gibt, sofern die Anforderungen erfüllt sind. Dieser Vertrag kann ausdrücklich, durch Dokumentation und / oder Behauptungen oder implizit sein, aber er besteht immer.

Teil des Vertrags Ihrer Klasse ist, dass der Aufrufer dem Konstruktor einige Argumente gibt, die nicht null sein dürfen. Die Durchführung des Vertrags liegt in der Verantwortung der Klasse. Die Prüfung, ob der Anrufer seinen Teil des Vertrags erfüllt hat, fällt leicht in den Verantwortungsbereich der Klasse.

Die Idee, dass eine Klasse einen Vertrag umsetzt, stammt von Bertrand Meyer , dem Designer der Programmiersprache Eiffel und der Idee, Design by Contract zu entwickeln . Die Eiffel-Sprache macht die Spezifikation und Überprüfung des Vertrages Teil der Sprache.

Wie bereits in anderen Antworten erwähnt, wird SRP häufig missverstanden. Es geht nicht darum, atomaren Code zu haben, der nur eine Funktion erfüllt. Es geht darum, sicherzustellen, dass Ihre Objekte und Methoden nur eine Sache tun und dass die eine Sache nur an einem Ort getan wird.

Schauen wir uns ein schlechtes Beispiel im Pseudocode an.

class Math
    private int a;
    private int b;
    def constructor(int x, int y) 
        if(x != null)
          a = x
        else if(x < 0)
          a = abs(x)
        else if (x == -1)
          throw "Some Silly Error"
        else
          a = 0
        end
        if(y != null)
           b = y
        else if(y < 0)
           b = abs(y)
        else if(y == -1)
           throw "Some Silly Error"
        else
         b = 0
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

In unserem absurden Beispiel besteht die "Verantwortung" des Math # -Konstruktors darin, das Math-Objekt verwendbar zu machen. Dies geschieht, indem zuerst die Eingabe bereinigt und dann sichergestellt wird, dass die Werte nicht -1 sind.

Dies ist eine gültige SRP, da der Konstruktor nur eines tut. Es bereitet das Math-Objekt vor. Es ist jedoch nicht sehr wartbar. Es verletzt DRY.

Also lasst es uns noch einmal versuchen

class Math
    private int a;
    private int b;
    def constructor(int x, int y)
        cleanX(x)
        cleanY(y)
    end
    def cleanX(int x)
        if(x != null)
          a = x
        else if(x < 0)
          a = abs(x)
        else if (x == -1)
          throw "Some Silly Error"
        else
          a = 0
        end
   end
   def cleanY(int y)
        if(y != null)
           b = y
        else if(y < 0)
           b = abs(y)
        else if(y == -1)
           throw "Some Silly Error"
        else
         b = 0
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

In diesem Pass sind wir ein bisschen besser in Bezug auf DRY geworden, aber wir haben immer noch einiges zu tun mit DRY. SRP auf der anderen Seite scheint ein bisschen aus. Wir haben jetzt zwei Funktionen mit dem gleichen Job. Sowohl cleanX als auch cleanY bereinigen die Eingabe.

Lass es uns noch einmal versuchen

class Math
    private int a;
    private int b;
    def constructor(int x, int y)
        a = clean(x)
        b = clean(y)
    end
    def clean(int i)
        if(i != null)
          return i
        else if(i < 0)
          return abs(i)
        else if (i == -1)
          throw "Some Silly Error"
        else
          return 0
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

Jetzt war endlich DRY besser dran, und SRP scheint sich einig zu sein. Wir haben nur einen Ort, an dem wir den Job desinfizieren.

Der Code ist theoretisch besser zu warten und noch besser. Wenn wir den Fehler beheben und den Code verschärfen, müssen wir ihn nur an einer Stelle ausführen.

class Math
    private int a;
    private int b;
    def constructor(int x, int y)
        a = clean(x)
        b = clean(y)
    end
    def clean(int i)
        if(i == null)
          return 0
        else if (i == -1)
          throw "Some Silly Error"
        else
          return abs(i)
        end
    end
    def add()
        return a + b
    end
    def sub()
        return b - a
    end
end

In den meisten Fällen der realen Welt wären die Objekte komplexer und SRP würde auf eine Reihe von Objekten angewendet. Das Alter kann zum Beispiel Vater, Mutter, Sohn, Tochter sein. Anstatt also 4 Klassen zu haben, die das Alter ab dem Geburtsdatum ermitteln, gibt es eine Personenklasse, die das tut, und die 4 Klassen, die davon abgeleitet sind. Aber ich hoffe, dieses Beispiel hilft bei der Erklärung. Bei SRP geht es nicht um atomare Aktionen, sondern darum, dass ein "Job" erledigt wird.

Apropos SRP: Onkel Bob mag keine überall verstreuten Nullschecks. Im Allgemeinen sollten Sie als Team die Verwendung von Null-Parametern für Konstruktoren nach Möglichkeit vermeiden. Wenn Sie Ihren Code außerhalb Ihres Teams veröffentlichen, können sich die Dinge ändern.

Das Erzwingen der Nichtnullierbarkeit von Konstruktorparametern, ohne zuvor die Kohäsion der betreffenden Klasse sicherzustellen, führt zu einer Aufblähung des aufrufenden Codes, insbesondere von Tests.

Wenn Sie solche Verträge wirklich durchsetzen möchten, sollten Sie die Verwendung von Debug.Assertoder etwas Ähnliches in Betracht ziehen , um die Unordnung zu verringern:

public AClassThatDefinitelyNeedsAWritableStream(Stream stream)
{
   Assert.That(stream.CanWrite, "Put crucial information here, and not inane bloat.");

   // Go on normal operation.
}