HTTP Response/Request Class

Posted on

Problem

This is just a simple class that you can you use to send requests and get the HTML code. I would like to get feedback on this.

For your request you can set:

  • URL
  • Request method: GET or POST
  • Request parameters: if you need to pass parameters
  • Request cookie

As a response you will get:

  • HTML
  • Response cookie

Request

class Request
{
    public string Url { get; set; }
    public string RequestMethod { get; set; }
    public string RequestParameters { get; set; }
    public string RequestCookie { get; set; }
    public string ResponseCookie { get; private set; }

    /// <summary>
    /// Send Request To URL
    /// </summary>
    /// <returns>HTML Text of URL</returns>
    public string SendRequest()
    {
        try
        {
            HttpWebRequest request = (HttpWebRequest)WebRequest.Create(Url);

            request.Method = RequestMethod;
            request.UserAgent = "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4";

            if (RequestCookie != null)
            {
                request.Headers["Cookie"] = RequestCookie;
            }

            WebProxy myProxy = new WebProxy("localhost", 8888);
            request.Proxy = myProxy;

            if (RequestParameters != null)
            {
                byte[] bytes = Encoding.UTF8.GetBytes(RequestParameters);
                request.ContentLength = bytes.Length;

                Stream requestStream = request.GetRequestStream();
                requestStream.Write(bytes, 0, bytes.Length);
            }

            HttpWebResponse response = (HttpWebResponse)request.GetResponse();
            response.Headers["Set-Cookie"] = ResponseCookie;

            StreamReader reader = new StreamReader(response.GetResponseStream());

            return reader.ReadToEnd();
        }
        catch (WebException ex)
        {
            StreamReader streamReader = new StreamReader(ex.Response.GetResponseStream());

            return streamReader.ReadToEnd();
        }
    }
}

Example

static void Main(string[] args)
    {
        Request myRequest = new Request();

        myRequest.Url = "https://google.com";
        myRequest.RequestMethod = "GET";

        string html = myRequest.SendRequest();

        Console.WriteLine("REQUEST DONE !");

        Console.WriteLine(html);

        Console.ReadKey();
    }

Solution

I don’t quite see the purpose of your class. You are basically taking an existing class from the .NET Framework and wrapping it up in something that only exposes a fraction of the functionality.

The code also relies on a lot of hard-coded settings. While specifying a specific user-agent might not cause too much concern, always using a specific HTTP proxy will.

You provide the results of the request in two ways. The actual HTML is returned as a string from the method (what if the site returns binary content?), while the cookies are set in a property. It’s strange to have a class called Request that contains a property starting with Response. The approach taken by the HttpWebRequest is much cleaner as it returns a whole class that contains all the information of the reply.

Apart from @DJurcau’s answer (with which I agree):

  1. You’re not closing your StreamReaders.

  2. /// <returns>HTML Text of URL</returns>

    Not always. It can also return an error.

  3. Exposing crucial request settings as public properties (rather than eg. method or constructor parameters) is questionable too, in my opinion. It doesn’t enforce calling code to supply the necessary minimum of information required for the request to succeed. So it’s not a particularly friendly API, if I the burden of remembering which properties need to be set is still on me.

    Your SendRequest method doesn’t even validate it explicitly – eg. if Url is not set, I’ll just get an exception from WebRequest complaining about the lack of requestUriString, and you’re leaving it up to me to figure out that it translates to the Url not being set. Same with RequestMethod.

    Such mutability also introduces an inherent lack of thread safety – what if someone changes one of these properties (from another thread) while SendRequest is being executed and is half-way through? We don’t always need thread safety, but it’s one thing not to implement it at all, sort of ignoring the issue altogether, and another: to throw it out of the window by design, for no good reason. Other things being equal, I’d say prefer stateless/immutable objects to stateful/mutable ones.

  4. Naming: if the class is named Request already, there’s little point in naming all its properties RequestMethod, RequestParameters, RequestCookie etc. (with curious omission of Url – why not RequestUrl, then?). It’s known as Smurf naming convention anti-pattern.

Leave a Reply

Your email address will not be published. Required fields are marked *