Validierung von Konstruktorparametern in C # - Best Practices

34

Was ist die beste Vorgehensweise für die Validierung von Konstruktorparametern?

Angenommen, ein einfaches Stück C #:

public class MyClass
{
    public MyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            throw new ArgumentException("Text cannot be empty");

        // continue with normal construction
    }
}

Wäre es akzeptabel, eine Ausnahme auszulösen?

Die Alternative, auf die ich gestoßen bin, war die Vorvalidierung vor der Instanziierung:

public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
        {
            MessageBox.Show("Text cannot be empty");
            return null;
        }
        else
        {
            return new MyClass(text);
        }
    }
}
MPelletier
quelle
10
"akzeptabel, um eine Ausnahme zu werfen?" Was ist die Alternative?
S.Lott
10
@S.Lott: Tu nichts. Legen Sie einen Standardwert fest. Drucken Sie eine Meldung in die Konsole / Protokolldatei. Eine Glocke läuten. Blink ein Licht.
FrustratedWithFormsDesigner
3
@FrustratedWithFormsDesigner: "Nichts tun?" Wie wird die Bedingung "Text nicht leer" erfüllt? Msgstr "Standardwert setzen"? Wie wird das Textargument verwendet? Die anderen Ideen sind in Ordnung, aber diese beiden würden zu einem Objekt in einem Zustand führen, der die Einschränkungen dieser Klasse verletzt.
S.Lott
1
@S.Lott Aus Angst, mich zu wiederholen, war ich offen für die Möglichkeit eines anderen Ansatzes, den ich nicht kannte. Ich glaube, ich weiß nicht alles, soviel weiß ich sicher.
MPelletier
3
@MPelletier: Wenn Sie die Parameter vorzeitig validieren, wird DRY verletzt. (Wiederholen Sie sich nicht) Da Sie diese Vorprüfung in jeden Code kopieren müssen, der versucht, diese Klasse zu instanziieren.
CaffGeek

Antworten:

26

Ich neige dazu, alle meine Validierungen im Konstruktor durchzuführen. Dies ist ein Muss, da ich fast immer unveränderliche Objekte erstelle. Für Ihren speziellen Fall halte ich das für akzeptabel.

if (string.IsNullOrEmpty(text))
    throw new ArgumentException("message", "text");

Wenn Sie .NET 4 verwenden, können Sie dies tun. Dies hängt natürlich davon ab, ob Sie eine Zeichenfolge, die nur Leerzeichen enthält, für ungültig halten.

if (string.IsNullOrWhiteSpace(text))
    throw new ArgumentException("message", "text");
ChaosPandion
quelle
Ich bin mir nicht sicher, ob sein "spezifischer Fall" spezifisch genug ist, um solche spezifischen Ratschläge zu geben. Wir haben keine Ahnung, ob es in diesem Zusammenhang sinnvoll ist , eine Ausnahme auszulösen oder einfach zuzulassen, dass das Objekt trotzdem existiert.
FrustratedWithFormsDesigner
@FrustratedWithFormsDesigner - Ich nehme an, Sie haben mich dafür abgewählt? Natürlich haben Sie Recht, dass ich extrapoliere, aber dieses theoretische Objekt muss eindeutig ungültig sein, wenn der Eingabetext leer ist. Möchten Sie wirklich aufrufen müssen Init, um eine Art Fehlercode zu erhalten, wenn eine Ausnahme genauso gut funktioniert, und Ihr Objekt dazu zwingen, immer einen gültigen Status beizubehalten?
ChaosPandion
2
Ich neige dazu, diesen ersten Fall in zwei separate Ausnahmen aufzuteilen: eine, die auf Nichtigkeit prüft und eine auslöst, und eine ArgumentNullExceptionandere, die auf leer prüft und eine auslöst ArgumentException. Ermöglicht es dem Anrufer, wählerischer zu sein, wenn er die Ausnahmebehandlung durchführen möchte.
Jesse C. Slicer
1
@Jesse - Ich habe diese Technik verwendet, bin aber immer noch am Zaun, was ihre allgemeine Nützlichkeit angeht.
ChaosPandion
3
+1 für unveränderliche Objekte! Ich benutze sie auch wo immer möglich (ich persönlich finde fast immer).
22

Viele Leute sagen, dass Konstrukteure keine Ausnahmen werfen sollten. KyleG auf dieser Seite macht zum Beispiel genau das. Ehrlich gesagt, ich kann mir keinen Grund vorstellen, warum nicht.

In C ++ ist es eine schlechte Idee, eine Ausnahme von einem Konstruktor auszulösen, da Ihnen dadurch Speicher zugewiesen wird, der ein nicht initialisiertes Objekt enthält, auf das Sie keinen Bezug haben (dh es ist ein klassischer Speicherverlust). Das Stigma kommt wahrscheinlich von dort - eine Reihe von C ++ - Entwicklern der alten Schule haben ihr C # -Lernen zur Hälfte abgebrochen und nur das angewendet, was sie von C ++ kannten. Im Gegensatz dazu hat Apple in Objective-C den Zuweisungsschritt vom Initialisierungsschritt getrennt, sodass Konstruktoren in dieser Sprache Ausnahmen auslösen können .

C # kann keinen Speicher von einem erfolglosen Aufruf an einen Konstruktor verlieren. Sogar einige Klassen im .NET Framework lösen Ausnahmen in ihren Konstruktoren aus.

Ameise
quelle
Sie werden so einige Federn mit Ihrem C ++ - Kommentar kräuseln. :)
ChaosPandion
5
Ehh, C ++ ist eine großartige Sprache, aber eine meiner Lieblingsbeschwerden sind Menschen, die eine Sprache gut lernen und dann die ganze Zeit so schreiben . C # ist nicht C ++.
Ant
10
Es kann immer noch gefährlich sein, Ausnahmen von einem ctor in C # auszulösen, da der ctor möglicherweise nicht verwaltete Ressourcen zugewiesen hat , die dann niemals verworfen werden. Der Finalizer muss so geschrieben werden, dass er das Szenario abwehrt, in dem ein unvollständig konstruiertes Objekt finalisiert wird. Diese Szenarien sind jedoch relativ selten. In einem kommenden Artikel über das C # -Entwicklungszentrum werden diese Szenarien und das defensive Umgehen mit Code behandelt. Sehen Sie sich diesen Abschnitt für Details an.
Eric Lippert
6
wie? eine Ausnahme von dem Ctor werfen ist das einzige , was man in c tun kann ++ , wenn Sie das Objekt nicht konstruieren können, und es gibt keinen Grund , dies hat einen Speicherverlust verursachen (obwohl seine offensichtlich bis zu).
jk.
@jk: Hmm, du hast recht! Obwohl es eine Alternative zu sein scheint, schlechte Objekte als "Zombies" zu markieren , was die STL anscheinend tut, anstatt Ausnahmen zu werfen: yosefk.com/c++fqa/exceptions.html#fqa-17.2 Die Lösung von Objective-C ist viel aufgeräumter.
Ameise
12

Eine Ausnahme auslösen, wenn die Klasse in Bezug auf ihre semantische Verwendung nicht in einen konsistenten Zustand versetzt werden kann. Ansonsten nicht. NIEMALS zulassen, dass ein Objekt in einem inkonsistenten Zustand existiert. Dazu gehört, dass keine vollständigen Konstruktoren bereitgestellt werden (z. B. ein leerer Konstruktor + initialize (), bevor Ihr Objekt tatsächlich vollständig erstellt wurde) ... SAGEN SIE NUR NEIN!

Zur Not macht es jeder. Ich habe es neulich für ein sehr eng genutztes Objekt in einem engen Rahmen gemacht. Eines Tages werde ich oder jemand anderes wahrscheinlich den Preis für diesen Zettel in der Praxis bezahlen.

Ich sollte beachten, dass mit "Konstruktor" das gemeint ist, was der Client aufruft, um das Objekt zu erstellen. Das könnte genauso gut etwas anderes als ein tatsächliches Konstrukt sein, das den Namen "Konstruktor" trägt. Zum Beispiel würde so etwas in C ++ das Prinzip IMNSHO nicht verletzen:

struct funky_object
{
  ...
private:
  funky_object();
  bool initialize(std::string);

  friend boost::optional<funky_object> build_funky(std::string);
};
boost::optional<funky_object> build_funky(std::string str)
{
  funky_object fo;
  if (fo.initialize(str)) return fo;
  return boost::optional<funky_object>();
}

Da der einzige Weg, ein Objekt zu erstellen, darin funky_objectbesteht, build_funkydas Prinzip aufzurufen, niemals zuzulassen, dass ein ungültiges Objekt existiert, bleibt es intakt, obwohl der eigentliche "Konstruktor" den Job nicht beendet.

Das ist eine Menge zusätzlicher Arbeit für fragwürdigen Gewinn (vielleicht sogar Verlust). Ich würde immer noch die Ausnahmeroute bevorzugen.

Edward Strange
quelle
9

In diesem Fall würde ich die Factory-Methode verwenden. Grundsätzlich sollten Sie festlegen, dass Ihre Klasse nur private Konstruktoren und eine Factory-Methode hat, die eine Instanz Ihres Objekts zurückgibt. Wenn die Anfangsparameter ungültig sind, geben Sie einfach null zurück und lassen Sie den aufrufenden Code entscheiden, was zu tun ist.

public class MyClass
{
    private MyClass(string text)
    {
        //normal construction
    }

    public static MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            return null;
        else
            return new MyClass(text);
    }
}
public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        var cls = MyClass.MakeMyClass(text);
        if(cls == null)
             //show messagebox or throw exception
        return cls;
    }
}

Werfen Sie niemals Ausnahmen, es sei denn, die Bedingungen sind außergewöhnlich. Ich denke, dass in diesem Fall ein leerer Wert leicht übergeben werden kann. Wenn dies der Fall ist, werden durch die Verwendung dieses Musters Ausnahmen und die damit verbundenen Leistungseinbußen vermieden, während der MyClass-Status gültig bleibt.

Donn Relacion
quelle
1
Ich sehe den Vorteil nicht. Warum ist es besser, das Ergebnis einer Factory auf null zu testen, als irgendwo einen try-catch zu haben, der die Ausnahme vom Konstruktor abfängt? Ihr Ansatz scheint zu sein, "Ausnahmen nicht zu berücksichtigen, sondern den Rückgabewert von Methoden explizit auf Fehler zu überprüfen". Wir haben vor langer Zeit akzeptiert, dass Ausnahmen in der Regel überlegen sind, jeden Rückgabewert einer Methode explizit auf Fehler zu prüfen, sodass Ihr Rat verdächtig erscheint. Ich würde gerne eine Begründung dafür sehen, warum Ihrer Meinung nach die allgemeine Regel hier nicht zutrifft.
DW
Wir haben nie akzeptiert, dass Ausnahmen den Rückgabecodes überlegen sind. Wenn sie zu oft geworfen werden, können sie zu einem schwerwiegenden Leistungseinbruch führen. Wenn Sie jedoch eine Ausnahme von einem Konstruktor auslösen, geht auch in .NET Speicher verloren, wenn Sie etwas zugewiesen haben, das nicht verwaltet wird. Sie müssen eine Menge Arbeit in Ihrem Finalizer erledigen, um diesen Fall auszugleichen. Auf jeden Fall ist die Factory hier eine gute Idee, und TBH sollte eine Ausnahme auslösen, anstatt eine Null zurückzugeben. Angenommen, Sie erstellen nur wenige "schlechte" Klassen, dann ist der Factory-Wurf vorzuziehen.
gbjbaanb
2
  • Ein Konstruktor sollte keine Nebenwirkungen haben.
    • Alles andere als die Initialisierung privater Felder sollte als Nebeneffekt angesehen werden.
    • Ein Konstruktor mit Nebenwirkungen bricht das Single-Responsibilty-Prinzip (SRP) und widerspricht dem Geist der objektorientierten Programmierung (OOP).
  • Ein Konstruktor sollte leicht sein und niemals versagen.
    • Zum Beispiel schaudere ich immer, wenn ich einen Try-Catch-Block in einem Konstruktor sehe. Ein Konstruktor sollte keine Ausnahmen auslösen oder Fehler protokollieren.

Man könnte diese Richtlinien vernünftigerweise in Frage stellen und sagen: "Aber ich halte mich nicht an diese Regeln und mein Code funktioniert einwandfrei!" Darauf würde ich antworten: "Nun, das mag wahr sein, bis es nicht mehr so ​​ist."

  • Ausnahmen und Fehler innerhalb eines Konstruktors sind sehr unerwartet. Wenn sie nicht dazu aufgefordert werden, werden zukünftige Programmierer nicht geneigt sein, diese Konstruktoraufrufe mit defensivem Code zu umgeben.
  • Wenn in der Produktion ein Fehler auftritt, kann es schwierig sein, die generierte Stapelverfolgung zu analysieren. Der obere Rand der Stapelablaufverfolgung zeigt möglicherweise auf den Konstruktoraufruf, aber im Konstruktor passieren viele Dinge, und er zeigt möglicherweise nicht auf das tatsächliche LOC, bei dem ein Fehler aufgetreten ist.
    • Ich habe viele .NET-Stack-Traces analysiert, bei denen dies der Fall war.
Jim G.
quelle
0

Es kommt darauf an, was MyClass macht. Wenn MyClass tatsächlich eine Datenrepository-Klasse und Parametertext eine Verbindungszeichenfolge ist, empfiehlt es sich, eine ArgumentException auszulösen. Wenn MyClass jedoch (zum Beispiel) die StringBuilder-Klasse ist, können Sie sie einfach leer lassen.

Es kommt also darauf an, wie wichtig der Parametertext für die Methode ist. Ist das Objekt mit einem Null- oder Leerwert sinnvoll?

Steven Striga
quelle
2
Ich denke, die Frage impliziert, dass die Klasse tatsächlich erfordert, dass die Zeichenfolge nicht leer ist. Dies ist in Ihrem StringBuilder-Beispiel nicht der Fall.
Sergio Acosta
Dann muss die Antwort ja sein - es ist akzeptabel, eine Ausnahme auszulösen. Um zu vermeiden, dass Sie diesen Fehler versuchen oder abfangen müssen, können Sie ein Factory-Muster verwenden, um die Objekterstellung für Sie durchzuführen, das einen leeren String-Fall enthält.
Steven Striga
0

Ich bevorzuge es, einen Standard festzulegen, aber ich weiß, dass Java die "Apache Commons" -Bibliothek hat, die so etwas tut, und ich denke, das ist auch eine ziemlich gute Idee. Ich sehe kein Problem beim Auslösen einer Ausnahme, wenn der ungültige Wert das Objekt in einen unbrauchbaren Zustand versetzen würde. Eine Saite ist kein gutes Beispiel, aber was ist, wenn es sich um die DI eines armen Mannes handelt? Sie könnten nicht arbeiten, wenn ein Wert von nullanstelle einer ICustomerRepositorySchnittstelle übergeben würde. In einer Situation wie dieser ist das Auslösen einer Ausnahme die richtige Vorgehensweise, würde ich sagen.

Wayne Molina
quelle
-1

Als ich in c ++ gearbeitet habe, habe ich keine Ausnahme im Konstruktor ausgelöst, da dies zu einem Speicherverlust führen kann. Ich hatte es auf die harte Tour gelernt. Jeder, der mit c ++ arbeitet, weiß, wie schwierig und problematisch ein Speicherverlust sein kann.

Wenn Sie sich jedoch in c # / Java befinden, haben Sie dieses Problem nicht, da der Garbage Collector den Speicher sammelt. Wenn Sie C # verwenden, ist es meiner Meinung nach vorzuziehen, die Eigenschaft zu verwenden, um sicherzustellen, dass Dateneinschränkungen garantiert sind.

public class MyClass
{
    private string _text;
    public string Text 
    {
        get
        {
            return _text;
        } 
        set
        {
            if (string.IsNullOrWhiteSpace(value))
                throw new ArgumentException("Text cannot be empty");
            _text = value;
        } 
    }

    public MyClass(string text)
    {
        Text = text;
        // continue with normal construction
    }
}
Ajitdh
quelle
-3

Das Auslösen einer Ausnahme ist für die Benutzer Ihrer Klasse eine echte Belastung. Wenn die Validierung ein Problem darstellt, wird das Erstellen des Objekts im Allgemeinen in zwei Schritten ausgeführt.

1 - Erstellen Sie die Instanz

2 - Rufen Sie eine Initialize-Methode mit einem Rückgabeergebnis auf.

ODER

Rufen Sie eine Methode / Funktion auf, die die Klasse erstellt, die die Validierung durchführen kann.

ODER

Haben Sie eine isValid-Flagge, die ich nicht besonders mag, aber einige Leute tun.

Dunk
quelle
3
@Dunk, das ist einfach falsch.
CaffGeek
4
@Chad, das ist deine Meinung, aber meine Meinung ist nicht falsch. Es ist nur anders als bei Ihnen. Ich finde Try-Catch-Code viel schwieriger zu lesen, und ich habe gesehen, dass viel mehr Fehler erzeugt wurden, wenn Leute Try-Catch für die einfache Fehlerbehandlung verwenden als herkömmliche Methoden. Das war meine Erfahrung und es ist nicht falsch. Es ist was es ist.
Dunk
5
Der PUNKT ist, dass, nachdem ich einen Konstruktor aufgerufen habe, das eine AUSNAHME ist, wenn es nicht erstellt wird, weil eine Eingabe ungültig ist. Die Daten der App befinden sich nun in einem inkonsistenten / ungültigen Zustand, wenn ich das Objekt einfach erstelle und ein Flag setze, das sagt isValid = false. Nach dem Erstellen einer Klasse testen zu müssen, ob sie gültig ist, ist ein schreckliches, sehr fehleranfälliges Design. Und, sagten Sie, haben Sie einen Konstruktor und rufen Sie dann initialize auf ... Was, wenn ich nicht initialize aufrufe? Was dann? Und was ist, wenn Initialize fehlerhafte Daten erhält? Kann ich jetzt eine Ausnahme auslösen? Ihre Klasse sollte nicht schwer zu bedienen sein.
CaffGeek
5
@Dunk, wenn Sie Ausnahmen als schwierig empfinden, verwenden Sie sie falsch. Einfach ausgedrückt, wurde einem Konstruktor eine falsche Eingabe bereitgestellt. Das ist eine Ausnahme. Die App sollte nicht weiter ausgeführt werden, es sei denn, es ist behoben. Zeitraum. Wenn dies der Fall ist, haben Sie ein schlimmeres Problem, schlechte Daten. Wenn Sie nicht wissen, wie Sie mit der Ausnahme umgehen sollen, lassen Sie sie den Aufrufstapel sprudeln, bis sie bearbeitet werden kann, auch wenn dies lediglich das Protokollieren und Stoppen der Ausführung bedeutet. Einfach ausgedrückt, müssen Sie keine Ausnahmen behandeln oder auf sie testen. Sie arbeiten einfach.
CaffGeek
3
Tut mir leid, das ist einfach zu viel eines Anti-Patterns, um es in Betracht zu ziehen.
MPelletier,