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();
}
2 Answers 2
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):
You're not closing your
StreamReader
s./// <returns>HTML Text of URL</returns>
Not always. It can also return an error.
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. ifUrl
is not set, I'll just get an exception fromWebRequest
complaining about the lack ofrequestUriString
, and you're leaving it up to me to figure out that it translates to theUrl
not being set. Same withRequestMethod
.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.Naming: if the class is named
Request
already, there's little point in naming all its propertiesRequestMethod
,RequestParameters
,RequestCookie
etc. (with curious omission ofUrl
- why notRequestUrl
, then?). It's known as Smurf naming convention anti-pattern.