2009-07-09 8 views
1

Ich kodiere eine einfache kleine Klasse mit einer einzigen Methode eine E-Mail senden. Mein Ziel ist es, es in einem älteren Visual Basic 6-Projekt zu implementieren und es als COM-Objekt über die COM-Interop-Funktion verfügbar zu machen.Ausnahmebehandlung: Wie granular würden Sie bei der Validierung von Argumenten vorgehen?

Es gibt ein Detail, das ich schwierig zu lösen finde, wie granular ich sein sollte, um Parameter zu validieren. An diesem Licht, ich eine Sache bin wirklich nicht glücklich darüber, und das ist kein Detail überhaupt, ist die Art, wie ich bin Ausnahme tatsächlich Handhabung:

public class MyMailerClass 
{ 
    #region Creation 
    public void SendMail(string from, string subject, string to, string body) 
    { 
     if (this.IsValidMessage(from, subject, to, body)) // CS1501 
     { 
      MailMessage msg = new MailMessage(); 
      msg.IsBodyHtml = true; 
      msg.From = new MailAddress(from); 
      msg.To.Add(to); 
      msg.Subject = subject; 
      msg.Body = body; 
      SmtpClient srv = new SmtpClient("SOME-SMTP-HOST.COM"); 
      srv.Send(msg); 
     } 
     else 
     { 
      throw new ApplicationException("Invalid message format."); 
     } 
    } 
    #endregion Creation 

    #region Validation 
    private bool IsValidMessage(string from, string subject, string to, string body) 
    { 
     Regex chk = new Regex(@"(\[email protected][a-zA-Z_]+?\.[a-zA-Z]{2,6})"); 
     if (!chk.IsMatch(from)) 
     { 
      return false; 
     } 
     if (!chk.IsMatch(to)) 
     { 
      return false; 
     } 
     if (!string.IsNullOrEmpty(subject)) 
     { 
      return false; 
     } 
     if (!string.IsNullOrEmpty(body)) 
     { 
      return false; 
     } 
     else 
     { 
      return true; 
     } 
    } 
    #endregion Validation 
} 

Jeder Vorschlag wird sehr geschätzt werden, so vielen Dank im Voraus für alle Ihre Kommentare!

Hinweis: Wäre es praktisch, Enterprise Library Validation Application Block in diesem speziellen Fall zu implementieren?

+0

Als Randbemerkung, Sie Argument_Exception falsch verwenden - das zweite Argument ist ein _string_, das der _name_ des ungültigen Arguments sein sollte. Stattdessen übergeben Sie den Wert des Arguments dort. Sie sollten etwas tun wie: 'werfen neue ArgumentException (" Ungültige Absenderadresse: "+ von," von ");' –

+0

Vielen Pavel, ich füge es hinzu! –

+0

Die neue Version des Codes wird schwer zu debuggen sein. In IsValidMessage() überprüfen Sie alle Bedingungen in einer Zeile. Wie finden Sie heraus, welcher nicht zufrieden ist, wenn Sie mit einem Debugger über den Code gehen? Sie könnten Folgendes schreiben: if (string.IsNullOrEmpty (subject)) {return false; } if (! string.IsNullOrEmpty (body)) {return false; } dann erstellen Sie ein Regex-Objekt und überprüfen Sie erneut eine Bedingung zu einem Zeitpunkt eine Rückgabe false sofort, sobald die Bedingung nicht erfüllt ist. – sharptooth

Antwort

9

Betrachten Sie den Vertrag, den Sie den Absendern von SendMail auferlegen. Sie müssen Ihnen eine "gültige E-Mail-Adresse" übermitteln. Wer entscheidet was gültig ist? SendMail tut es. Grundsätzlich ist Ihre Methode "hohe Wartung" - sie will Dinge genau so, wie sie es mag, und die einzige Möglichkeit zu sagen, ob das, was Sie geben werden, zufriedenstellend sein wird, ist zu versuchen, auf das Beste zu hoffen.

Schreiben Sie keine wartungsintensiven Methoden, ohne dem Anrufer die Möglichkeit zu geben, zu wissen, wie er sie erfüllen kann, oder zumindest eine Möglichkeit, die Ausnahme zu vermeiden. Extrahieren Sie die Validierungslogik in eine "IsValidAddress" -Methode, die einen Booleschen Wert zurückgibt. Lassen Sie dann Ihre SendMail-Methode IsValidAddress aufrufen und werfen Sie sie, wenn sie ungültig ist.

Sie erhalten einige nette Effekte aus dieser Änderung:

(1) erhöhte Trennung von Bedenken. Die Aufgabe von SendMail besteht darin, den E-Mail-Mechanismus zum Funktionieren zu bringen und nicht zu beurteilen, ob eine E-Mail-Adresse gültig ist. Isolieren Sie diese Richtlinienentscheidung auf Code, der auf die Überprüfung spezialisiert ist.

(2) Adressvalidierung ist ein nützliches Werkzeug an sich; Es gibt viele Male, wenn Sie wissen möchten, ob eine Adresse wohlgeformt ist, ohne eine Mail an sie zu senden.

(3) Sie können Ihre Validierungslogik einfach aktualisieren und verbessern, weil alles an einem sinnvollen Ort ist.

(4) Anrufer haben eine Möglichkeit, dass sie garantieren können, dass keine Ausnahme ausgelöst wird. Wenn ein Aufrufer eine Methode nicht aufrufen kann, ohne sicherzustellen, dass die Argumente gültig sind, müssen sie die Ausnahme abfangen. Im Idealfall sollten Sie niemals veranlassen, dass ein Aufrufer eine Ausnahme behandelt, um den Code korrekt zu machen. es sollte einen Weg geben, wie sie korrekten Code schreiben können, der niemals würfelt, selbst wenn die Daten, die sie bekommen haben, schlecht sind.

Hier sind ein paar Artikel, die ich zu diesem Thema geschrieben haben, die Sie nützlich finden könnten:

Ausnahmebehandlung: http://ericlippert.com/2008/09/10/vexing-exceptions/

Hochwartungsmethoden: http://blogs.msdn.com/ericlippert/archive/2008/09/08/high-maintenance.aspx

+0

Eric, ich möchte Ihnen für diesen ausgearbeiteten und fundierten Rat sehr danken! –

3

Mit zwei throw Aussagen in einer Reihe macht keinen Sinn - nur die erste ausgeführt wird, und dann wird die Steuerung an den Exception-Handler übergeben werden und nie auf die zweite throw.

Meiner Meinung nach ist es mehr als genug zu sagen, etw wie "Absender E-Mail ist ungültig." Eine E-Mail ist ziemlich einfach und kurz und so kann der Benutzer dies ohne zusätzliche Anleitung lösen.

Ich denke auch, es wäre besser, zuerst alle übergebenen Werte zu überprüfen und erst dann zu arbeiten. Was ist der Punkt bei der teilweisen Arbeit, wenn Sie dann auf einen ungültigen Parameterwert stoßen und eine Ausnahme auslösen können und diese Arbeit niemals abschließen? Versuchen Sie, Fehler so früh wie möglich anzuzeigen - ganz am Anfang, wenn möglich.

+0

Sie meinen also, dass Sie, vielleicht in ternärer Weise, die Gültigkeit jedes Parameters am Anfang der Methode überprüfen, eine Art boolesche Flagge in Form von "isMsgOk = true" bis zum Ende tragen und dann alles komponieren Dort? –

+0

Nein, warum verwenden Sie eine Flagge, wenn Sie Ausnahmen haben? Sie können jeden Parameter überprüfen und sobald Sie einen ersten ungültigen Wert gefunden haben, werfen Sie einfach eine Ausnahme aus. – sharptooth

1

Und:

Verwenden

string.IsNullOrEmpty(subject) 

statt

subject == null 

zum Prüfen, ob die Saiten leer sind.

+0

Danke für diesen Ausschnitt Jason! Ich schließe es ein. –