2010-02-01 13 views
6

Ist es in Ordnung, ein "try/catch" in eine "using" -Anweisung für eine Web-Anfrage einzubetten? Ist mein Code korrekt? Das ist meine Anforderungen sind:C# - ist es ok, ein "try/catch" in eine "using" -Anweisung für eine Web-Anfrage einzubetten? Ist mein Code korrekt?

  1. Möchten Sie die „mit“ Anweisung verwenden, dass Ressourcen zu machen, sind in jedem Fall freigegeben für HttpWebResponse

    • Aber wollen noch einige benutzerdefinierten Dinge zu tun, wenn es eine ist Ausnahme für HttpWebResponse und "response = (HttpWebResponse) request.GetResponse();" bestimmtes.

Mein Quellcode:

 var result = new HttpHeaderInfo(); 
     HttpWebRequest request = null; 
     HttpWebResponse response = null; 
     using (response) 
     { 
      try 
      { 
       request = (HttpWebRequest)WebRequest.Create(uri); 
       request.Method = "HEAD"; 
       request.KeepAlive = false; 
       request.Timeout = Properties.Settings.Default.WebTimeoutDefault; 

       response = (HttpWebResponse)request.GetResponse(); 
       result.LastModified = response.LastModified; 
       result.ContentType = response.ContentType; 
       result.StatusCode = response.StatusCode; 
       result.ContentLength = response.ContentLength; 
      } 
      catch (Exception ex) 
      { 
       if (ex is InvalidOperationException || 
        ex is ProtocolViolationException || 
        ex is WebException) 
       { 
        result.HttpError = ex; 
        result.LastModified = System.DateTime.MinValue; 
        result.ContentType = null; 
       } 
       else { throw; } 
      } 

     } 

dank

Antwort

9

Es ist in Ordnung, aber ein wenig überflüssig; In einem allgemeinen Sinne könnten Sie problemlos den using Block entfernen, einen finally Block nach dem catch hinzufügen und explizit Dispose dort aufrufen, was die Verschachtelung in Ihrem Code reduzieren würde.

In einem spezielleren Sinn, was nervt mich ein wenig, dass Sie tatsächlich response nicht zuweisen, bis Sie innerhalb den using Block zu erhalten, und die expliziten Variablendeklarationen sind unnötig und verwirrend in diesem Zusammenhang. Ich würde es umschreiben als:

HttpHeaderInfo result; 
try 
{ 
    var request = (HttpWebRequest)WebRequest.Create(uri); 
    request.Method = "HEAD"; 
    request.KeepAlive = false; 
    request.Timeout = Properties.Settings.Default.WebTimeoutDefault; 

    using (HttpWebResponse response = (HttpWebResponse)request.GetResponse()) 
    { 
     result = new HttpHeaderInfo(); 
     result.LastModified = response.LastModified; 
     result.ContentType = response.ContentType; 
     result.StatusCode = response.StatusCode; 
     result.ContentLength = response.ContentLength; 
    } 
} 
catch (WebException ex) 
{ 
    // etc. 
} 

Dies ist viel klarer als die ursprüngliche Form. Beachten Sie auch, dass ich WebException, nicht die generische System.Exception abfangen. Sie sollten bestimmte Ausnahmetypen abfangen, anstatt generische Ausnahmen abzufangen und dann deren Typ zu überprüfen.

+0

danke - haben Sie bemerkt, dass ich nur 3 spezifische Ausnahmen in meinem Code erfasst habe - ich war mir nicht sicher, inwieweit InvalidOperationException und ProtocolViolationException cuold als Systemtypfehler behandelt wurden (unwahrscheinlich, wenn der Code ein Problem hat) - Glaubst du, dass es für HTTPWebRequest hier angebracht ist, WebException nur dann zu behandeln? – Greg

+0

@Greg: Sie müssen wahrscheinlich die anderen Ausnahmen behandeln, ich wollte das Beispiel einfach nicht überladen. Um das zu tun, können Sie mehrere 'catch' Blöcke hinzufügen -" Moron's "Antwort zeigt ein Beispiel dafür. (Anmerkung - Ich würde wahrscheinlich "ProtocolViolationException", aber nicht "InvalidOperationException" abfangen - der erste kommt von letzterem und Sie sollten keinen anderen Typ von "InvalidOperationException" von "GetResponse" bekommen, nicht so weit ich weiß ...) – Aaronaught

1

Das ist völlig in Ordnung. Sie behandeln die Ausnahme und wollen nicht, dass es weiter sprudelt, das ist in Ordnung, und verschachtelte try/catch/finally-Blöcke sind kein Problem. (Intern ist ein 'Verwenden' wie das nur ein Versuch/endlich.)

UPDATE: lesen Sie ein wenig näher, und ich denke, dass Sie tatsächlich die Verwendung im 'try' Block wollen - die Linie, wo Sie tatsächlich ein Objekt In der Variable 'response' soll der Block 'using' beginnen. Kompiliert es tatsächlich wie es ist?

6

Andere haben dies aus als Potential Problem hingewiesen, aber ich will es als ganz bestimmtes Problem erhöhen: Ihre Anweisung using Sie überhaupt im Moment nicht gut tun.

Wenn Sie schreiben eine using-Anweisung wie folgt aus:

SomeType x = value1; 
using (x) 
{ 
    x = value2; 
} 

es ist value1, die am Ende des Blocks angeordnet werden, nichtvalue2. In Ihrem Code ist response Null bis in den Block; die WebResponse Sie enden mit wird nicht entsorgt werden.

Sie sollten eine Warnung darüber, in dieser Richtung sehen werden:

Warnung CS0728: Möglicherweise falsche Zuordnung zu den lokalen ‚Antwort‘, die das Argument für eine Verwendung oder lock-Anweisung ist. Der Aufruf von Dispose oder wird auf dem ursprünglichen Wert des lokalen freigegeben.

Diese Warnung ist wichtig - beachten Sie es.

dass Abgesehen, es ist ganz sinnvoll, einen try/catch-Block in einer using-Anweisung zu setzen ... aber in diesem Fall ist es wahrscheinlich außerhalb die Anweisung using sein sollte, so dass Sie die response Variable zu gegebener Zeit zu initialisieren so dass die Antwort immer entsorgt wird. Ich würde auch in Betracht ziehen, mehrere catch-Blöcke zu verwenden, die eine allgemeine Methode aufrufen, anstatt "is" wiederholt zu verwenden.