2
\$\begingroup\$

I am working on top of existing application and I have this (original) method for making a HttpWebRequest:

        internal string SendWebRequest(string url, Guid Id)
        {
            string responseFromServer = string.Empty;
            Uri asyncUri;
            if (Uri.TryCreate(url, UriKind.Absolute, out asyncUri))
            {
                HttpWebRequest webRequest = (HttpWebRequest) WebRequest.Create(asyncUri);
                webRequest.Method = "POST";
                webRequest.KeepAlive = false;
                webRequest.ContentType = "...";

                StringBuilder sbPostedData = new StringBuilder();
                sbPostedData.Append("ID").Append("=").Append(Base64Encode(Id.ToString()));

                ASCIIEncoding encoding = new ASCIIEncoding();
                byte[] postedData = encoding.GetBytes(sbPostedData.ToString());
                webRequest.ContentLength = postedData.Length;
                Stream requestStream = webRequest.GetRequestStream();
                requestStream.Write(postedData, 0, postedData.Length);
                requestStream.Close();

                WebResponse response = webRequest.GetResponse();

                var dataStream = response.GetResponseStream();
                StreamReader reader = new StreamReader(dataStream);
                responseFromServer = reader.ReadToEnd();
            }

            return responseFromServer;
        }

The things that got my attention here are that:

  1. The method is used only within the class but yet the access modifier is internal. However I'm having the same method with the same logic. For the time being I've made it private but maybe it's a candidate for some utility or helper class?
  2. We call WebResponse response = webRequest.GetResponse(); but actually when we get this - responseFromServer = reader.ReadToEnd(); and 3 lines below return it, the result is actually not used.

So my questions are, given the fact that the response is never used, first, is it fine to just get rid of this piece of code:

    WebResponse response = webRequest.GetResponse();

    var dataStream = response.GetResponseStream();
    StreamReader reader = new StreamReader(dataStream);
    responseFromServer = reader.ReadToEnd();

Just make the method void, and add this only when/if needed? What do you think is better, adding the same method in the new class and making it private or extracting it to some helper/utility class. And any suggestions on refactoring this code snippet?

\$\endgroup\$
  • 1
    \$\begingroup\$ Why wouldn't you be interested in verifying whether the request succeeded? \$\endgroup\$ – 200_success Mar 9 '17 at 8:30
  • \$\begingroup\$ @200_success Well, to be honest I'm not sure if in this exact scenario this would make any difference. This code is used from Quartz to make calls to another application. Maybe we could check that status of the request and maybe be repeat it if needed but it's not required. However it's a good question to ask, why we actually we don't do it. \$\endgroup\$ – Leron Mar 9 '17 at 10:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.