-1

Could someone please tell me if the code below is correct as far as "using","close" and "try-catch" is concerned? Not at all a C#/CLR guy, inherited a clunky piece of code and trying to get it sorted:

using System;
using System.Data;
using System.Data.SqlClient;
using System.Data.SqlTypes;
using System.Collections;
using System.Globalization;

// For the SQL Server integration
using Microsoft.SqlServer.Server;

// Other things we need for WebRequest
using System.Net;
using System.Text;
using System.IO;


public partial class UserDefinedFunctions
{

    // Function to return a web URL as a string value.
    [Microsoft.SqlServer.Server.SqlFunction(DataAccess = DataAccessKind.Read)]
    public static SqlString GET(SqlString uri, SqlString username, SqlString passwd)
    {
        // The SqlPipe is how we send data back to the caller
        SqlPipe pipe = SqlContext.Pipe;
        SqlString document;
        try
        {
            // Set up the request, including authentication
            WebRequest req = WebRequest.Create(Convert.ToString(uri));
            if (Convert.ToString(username) != null & Convert.ToString(username) != "")
            {
                req.Credentials = new NetworkCredential(
                    Convert.ToString(username),
                    Convert.ToString(passwd));
            }
            ((HttpWebRequest)req).UserAgent = "CLR web client on SQL Server";

            // Fire off the request and retrieve the response.
            using (WebResponse resp = req.GetResponse())
            {

                using (Stream dataStream = resp.GetResponseStream())
                {
                    //SqlContext.Pipe.Send("...get the data");
                    using (StreamReader rdr = new StreamReader(dataStream))
                    {
                        document = (SqlString)rdr.ReadToEnd();
                        rdr.Close();
                    }

                    // Close up everything...
                    dataStream.Close();
                }
                resp.Close();
                // .. and return the output to the caller.
                return (document);
            }//end using
        }
        catch (WebException e)
        {
            document = e.ToString();
            return (document);
            throw;
        }
    }

    // Function to submit a HTTP POST and return the resulting output.
    [Microsoft.SqlServer.Server.SqlFunction(DataAccess = DataAccessKind.Read)]
    public static SqlString POST(SqlString uri, SqlString postData, SqlString username, SqlString passwd)
    {
        SqlPipe pipe = SqlContext.Pipe;
        SqlString document;
        byte[] postByteArray = Encoding.UTF8.GetBytes(Convert.ToString(postData));

        // Set up the request, including authentication, 
        // method=POST and encoding:
        try
        {
            WebRequest req = WebRequest.Create(Convert.ToString(uri));
            ((HttpWebRequest)req).UserAgent = "CLR web client on SQL Server";
            if (Convert.ToString(username) != null & Convert.ToString(username) != "")
            {
                req.Credentials = new NetworkCredential(
                    Convert.ToString(username),
                    Convert.ToString(passwd));
            }
            req.Method = "POST";
            req.ContentType = "application/x-www-form-urlencoded";

            // Submit the POST data
            using (Stream dataStream = req.GetRequestStream())
            {
                dataStream.Write(postByteArray, 0, postByteArray.Length);
                dataStream.Close();
            }
            // Collect the response, put it in the string variable "document"
            using (WebResponse resp = req.GetResponse())
            {
                Stream dataStream = resp.GetResponseStream();
                using (StreamReader rdr = new StreamReader(dataStream))
                {
                    document = (SqlString)rdr.ReadToEnd();
                    rdr.Close();
                }
                dataStream.Close();
                resp.Close();
            }

            return (document);
        }//end try
        catch (WebException e)
        {
            document = e.ToString();
            return (document);
            throw;
        }//end catch
    }
}

I have been searching and checking every StackOverflow post but I just cannot seem to get this sorted in my head. Do I need "close" in each of the "using" blocks? Or do the "using" blocks call "Dispose" automatically and therefore make the "close" irrelevant?

Camilo Terevinto
  • 31,141
  • 6
  • 88
  • 120
A. Guattery
  • 113
  • 8
  • 1
    Standardly `Dispose` calls `Close`, and some code analysers even say, that you should not call Dispose twice, or that there is no need to call `Close`. – Julo May 12 '18 at 19:47
  • Not sure of intent but re-throwing exceptions is generally to be avoided IMO. I.e. `catch { ... throw; }` smells. – blins May 12 '18 at 19:55
  • Ok so then "close" is redundant and does not need to be called. @blins are you saying "throw" should not be there? Complete newb at this, trying to get it all together! And sorry for the possible duplicate here (I did read both of the post suggested, and many more). – A. Guattery May 12 '18 at 20:17
  • Regarding `throw;`, I don’t mean to say never use it but rather be careful. Here’s a good read: https://stackoverflow.com/questions/178456/what-is-the-proper-way-to-re-throw-an-exception-in-c – blins May 12 '18 at 20:35
  • Consider posting your question here: https://codereview.stackexchange.com – blins May 12 '18 at 20:37

1 Answers1

1

As stated in Microsoft using Statement documentation...

The using statement ensures that Dispose is called even if an exception occurs while you are calling methods on the object. You can achieve the same result by putting the object inside a try block and then calling Dispose in a finally block; in fact, this is how the using statement is translated by the compiler.

So, you shouldn't be calling Close or Dispose when using the using statement.

Franco Fusaro
  • 135
  • 1
  • 9