2
\$\begingroup\$

I've a C# WinForms application that relies heavily on sending HttpWebRequests. I've build an HttpWebRequestBuilder, and WebRequestBodyBuilder.

public class HttpWebRequestBuilder
{
    private HttpWebRequest _httpWebRequest; // The object that will be built.
    public HttpWebRequestBuilder(string url) : this(url, Constants.FirefoxUserAgent) // Use firefox user-agent by default (Constants is a static class that has FirefoxUserAgent constant).
    {
    }

    public HttpWebRequestBuilder(string url, string userAgent)
    {
        _httpWebRequest = WebRequest.CreateHttp(url); // Create an HttpWebRequest.
        _httpWebRequest.UserAgent = userAgent; // Set the user-agent.
        _httpWebRequest.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; // Set automatic decompression. I want this value for all requests.
    }

    public HttpWebRequestBuilder WithMethod(string method) // Set the method, "GET", "POST", etc. The default is "GET".
    {
        _httpWebRequest.Method = method;
        return this;
    }

    public HttpWebRequestBuilder Accepts(string accept) // Set the "Accept" header.
    {
        _httpWebRequest.Accept = accept;
        return this;
    }

    public HttpWebRequestBuilder WithContentType(string contentType) // Set the "Content-Type" header.
    {
        _httpWebRequest.ContentType = contentType;
        return this;
    }

    public HttpWebRequestBuilder WithCookies(CookieContainer container) // Set a cookie container.
    {
        _httpWebRequest.CookieContainer = container;
        return this;
    }

    public HttpWebRequestBuilder AllowAutoRedirect(bool allow) // Set AllowAutoRedirect
    {
        _httpWebRequest.AllowAutoRedirect = allow;
        return this;
    }

    public HttpWebRequestBuilder WithReferer(string referer) // Set referer.
    {
        _httpWebRequest.Referer = referer;
        return this;
    }

    public HttpWebRequestBuilder WithCustomHeader(string name, string value) // Add a custom header.
    {
        _httpWebRequest.Headers.Add(name, value);
        return this;
    }

    public async Task<HttpWebRequestBuilder> PostAsync(WebRequestBodyBuilder bodyBuilder) // Takes an object of WebRequestBodyBuilder (which just makes a string like "param1=value1&param2=value2&...."
    {
        byte[] buffer = Encoding.UTF8.GetBytes(bodyBuilder.ToString());
        using (var reqStrm = await _httpWebRequest.GetRequestStreamAsync()) // Write the body in the request stream.
        {
            reqStrm.Write(buffer, 0, buffer.Length);
        }
        return this;
    }

    public HttpWebRequest Build() // Returns the actual HttpWebRequest
    {
        return _httpWebRequest;
    }
}
public class WebRequestBodyBuilder
{
    private StringBuilder _stringBuilder = new StringBuilder();

    public WebRequestBodyBuilder SetKeyValuePair(string key, string value)
    {
        if (string.IsNullOrWhiteSpace(key))
            throw new ArgumentException($"Argument {nameof(key)} can't be null or whitespace."); // key can't be null, but value CAN!

        if (!string.IsNullOrWhiteSpace(_stringBuilder.ToString()))
            _stringBuilder.Append("&"); // First call shouldn't add "&" in the first.
        _stringBuilder.Append($"{WebUtility.UrlEncode(key)}={WebUtility.UrlEncode(value)}");
        return this;
    }

    public override string ToString()
    {
        return _stringBuilder.ToString(); // ToString is used in the HttpWebRequestBuilder, in the PostAsync method.
    }
}

An example usage of these is like the following:

var bodyBuilder = new WebRequestBodyBuilder()
    .SetKeyValuePair("username", Account.Username)
    .SetKeyValuePair("password", Account.Password)
    .SetKeyValuePair("auth_token", _token);


HttpWebRequest req = (await (new HttpWebRequestBuilder() // The await is for PostAsync.
    .WithMethod("POST")
    .WithReferer("https://website.com/login")
    .WithCookies(_cookies)
    .Accepts("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8")
    .WithContentType("application/x-www-form-urlencoded")
    .AllowAutoRedirect(false).PostAsync(bodyBuilder))).Build();
// Do something with req, like await req.GetResponseAsync()

I'm just starting learning design patterns, so I want to make sure that I'm not misusing them.

I'm aware that docs recommends HttpClient over HttpWebRequest, but this is another story. My question is specific about applying the builder pattern in this case

\$\endgroup\$
  • 2
    \$\begingroup\$ Why do you need this builder at all when you can simply initialize the properties? Part of learning design patterns is when NOT to use them because there are simpler alternatives. \$\endgroup\$ – Roland Illig Jan 21 at 20:15
  • \$\begingroup\$ @RolandIllig, The advantages I got are: • Having AutomaticDecompression and User-Agent set for all requests automatically. • The request body builder allows me for easier usage. • For me, it just looks easier, but I may be mistaken. \$\endgroup\$ – Youssef13 Jan 22 at 6:45
  • \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ – BCdotWEB Jan 23 at 12:00
  • \$\begingroup\$ @BCdotWEB, Does the new title look good to you? \$\endgroup\$ – Youssef13 Jan 23 at 12:03
  • \$\begingroup\$ @Youssef13 Like I said: the title should simply state the task accomplished by the code. Read the link I posted in my comment, which for instance contains this hint: "If your title contains a question, it is likely a bad title." \$\endgroup\$ – BCdotWEB Jan 23 at 12:22
4
\$\begingroup\$

Good implementation. Few minor bits:

  1. Mark _httpWebRequest as readonly as you're not assigning (nor want anyone to assign) to it outside of the constructor.

  2. Extract an interface (IHttpRequestBuilder) and have each method return that rather than the concrete class. This will allow for additional implementations or mocking for unit testing other parts of your system.

    2a. The same advice as above goes for WebRequestBodyBuilder.

  3. PostAsync is already an async method, so continue using async implementations like WriteAsync() instead of Write().

  4. Create constructors that take Uri parameters as well.

Results:

IHttpRequestBuilder.cs

public interface IHttpWebRequestBuilder
{
    IHttpWebRequestBuilder WithMethod(string method); // Set the method, "GET", "POST", etc. The default is "GET".

    IHttpWebRequestBuilder Accepts(string accept); // Set the "Accept" header.

    IHttpWebRequestBuilder WithContentType(string contentType); // Set the "Content-Type" header.

    IHttpWebRequestBuilder WithCookies(CookieContainer container); // Set a cookie container.

    IHttpWebRequestBuilder AllowAutoRedirect(bool allow); // Set AllowAutoRedirect

    IHttpWebRequestBuilder WithReferer(string referer); // Set referer.

    IHttpWebRequestBuilder WithCustomHeader(string name, string value); // Add a custom header.

    Task<IHttpWebRequestBuilder> PostAsync(IWebRequestBodyBuilder bodyBuilder); // Takes an object of WebRequestBodyBuilder (which just makes a string like "param1=value1&param2=value2&...."

    HttpWebRequest Build(); // Returns the actual HttpWebRequest
}

HttpRequestBuilder.cs

public class HttpWebRequestBuilder : IHttpWebRequestBuilder
{
    private readonly HttpWebRequest _httpWebRequest; // The object that will be built.
    public HttpWebRequestBuilder(string url) : this(url, Constants.FirefoxUserAgent) // Use firefox user-agent by default (Constants is a static class that has FirefoxUserAgent constant).
    {
    }

    public HttpWebRequestBuilder(Uri uri) : this(uri, Constants.FirefoxUserAgent) // Use firefox user-agent by default (Constants is a static class that has FirefoxUserAgent constant).
    {
    }

    public HttpWebRequestBuilder(string url, string userAgent) : this(new Uri(url), userAgent)
    {
    }

    public HttpWebRequestBuilder(Uri uri, string userAgent)
    {
        _httpWebRequest = WebRequest.CreateHttp(uri); // Create an HttpWebRequest.
        _httpWebRequest.UserAgent = userAgent; // Set the user-agent.
        _httpWebRequest.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; // Set automatic decompression. I want this value for all requests.
    }

    public IHttpWebRequestBuilder WithMethod(string method) // Set the method, "GET", "POST", etc. The default is "GET".
    {
        _httpWebRequest.Method = method;
        return this;
    }

    public IHttpWebRequestBuilder Accepts(string accept) // Set the "Accept" header.
    {
        _httpWebRequest.Accept = accept;
        return this;
    }

    public IHttpWebRequestBuilder WithContentType(string contentType) // Set the "Content-Type" header.
    {
        _httpWebRequest.ContentType = contentType;
        return this;
    }

    public IHttpWebRequestBuilder WithCookies(CookieContainer container) // Set a cookie container.
    {
        _httpWebRequest.CookieContainer = container;
        return this;
    }

    public IHttpWebRequestBuilder AllowAutoRedirect(bool allow) // Set AllowAutoRedirect
    {
        _httpWebRequest.AllowAutoRedirect = allow;
        return this;
    }

    public IHttpWebRequestBuilder WithReferer(string referer) // Set referer.
    {
        _httpWebRequest.Referer = referer;
        return this;
    }

    public IHttpWebRequestBuilder WithCustomHeader(string name, string value) // Add a custom header.
    {
        _httpWebRequest.Headers.Add(name, value);
        return this;
    }

    public async Task<IHttpWebRequestBuilder> PostAsync(IWebRequestBodyBuilder bodyBuilder) // Takes an object of WebRequestBodyBuilder (which just makes a string like "param1=value1&param2=value2&...."
    {
        byte[] buffer = Encoding.UTF8.GetBytes(bodyBuilder.ToString());
        using (var reqStrm = await _httpWebRequest.GetRequestStreamAsync()) // Write the body in the request stream.
        {
            await reqStrm.WriteAsync(buffer, 0, buffer.Length);
        }
        return this;
    }

    public HttpWebRequest Build() // Returns the actual HttpWebRequest
    {
        return _httpWebRequest;
    }
}

IWebRequestBodyBuilder.cs

public interface IWebRequestBodyBuilder
{
    IWebRequestBodyBuilder SetKeyValuePair(string key, string value);
}

WebRequestBodyBuilder.cs

public class WebRequestBodyBuilder : IWebRequestBodyBuilder
{
    private readonly StringBuilder _stringBuilder = new StringBuilder();

    public IWebRequestBodyBuilder SetKeyValuePair(string key, string value)
    {
        if (string.IsNullOrWhiteSpace(key))
            throw new ArgumentException($"Argument {nameof(key)} can't be null or whitespace."); // key can't be null, but value CAN!

        if (!string.IsNullOrWhiteSpace(_stringBuilder.ToString()))
            _stringBuilder.Append("&"); // First call shouldn't add "&" in the first.
        _stringBuilder.Append($"{WebUtility.UrlEncode(key)}={WebUtility.UrlEncode(value)}");
        return this;
    }

    public override string ToString()
    {
        return _stringBuilder.ToString(); // ToString is used in the HttpWebRequestBuilder, in the PostAsync method.
    }
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Good points! I'm only wondering the usage of interface, why would I need it if I have only one class that implements it? There is no scenario in mind that will make me implement another class. Unfortunately, "interfaces" concept is always confuses me, especially, when I see that only one class implements it. \$\endgroup\$ – Youssef13 Jan 21 at 17:16
  • \$\begingroup\$ That's probably a secondary reason to my other justification: being able to mock the interface (using, for example, Moq) when writing unit tests that use this class. It fosters the I and D in SOLID development principles. \$\endgroup\$ – Jesse C. Slicer Jan 21 at 17:26
  • 1
    \$\begingroup\$ Why not use the concrete class for unit testing? I was told that mocking for unit tests is used when the class does some database operations, so I want to avoid making changes to a production database. \$\endgroup\$ – Youssef13 Jan 21 at 17:32
  • 1
    \$\begingroup\$ That's really a longer question with a longer answer, but the simple answer is: you mock ALL dependencies so ONLY the class/unit you're testing is being exercised. \$\endgroup\$ – Jesse C. Slicer Jan 21 at 17:47
  • \$\begingroup\$ Mock a URL builder? That would mean you also have to mock String and int32, if I understand the word all correctly. \$\endgroup\$ – Roland Illig Jan 21 at 20:10
2
\$\begingroup\$

I have modified yours, (not tested), but I want to share it just to give you some insights, not the best design, but I felt I need to share it.

//singleton class
public sealed class HttpRequestClient
{
    internal enum MethodType
    {
        GET, POST, PUT, DELETE
    }

    internal enum RequestContentType { JSON, XML, x_www_form_urlencoded }

    private string Url;
    // for concurrent requests
    private static HttpWebRequest _request;

    private static HttpRequestClient _client;

    private HttpRequestHeaderBuilder HeaderBuilder;

    private HttpRequestMethodBuilder MethodBuilder;

    //body
    private StringBuilder _stringBuilder;

    public HttpRequestClient(string url)
    {
        if(string.IsNullOrEmpty(url)) { throw new ArgumentNullException(nameof(url)); }

        _client = _client ?? (_client = new HttpRequestClient(url));

        _request = _request = WebRequest.CreateHttp(new Uri(url)); // this will call the main method CreateHttp(Uri url) directly.

        _stringBuilder = new StringBuilder();
    }

    internal struct HttpRequestHeaderBuilder
    {
        internal HttpRequestHeaderBuilder Accepts(string accept) // Set the "Accept" header.
        {
            // do the same for the other methods, always validate the value beofre assign it.
            if (string.IsNullOrEmpty(accept)) { throw new ArgumentNullException(nameof(accept)); }

            _request.Accept = accept;

            return this;
        }

        internal HttpRequestHeaderBuilder ContentType(RequestContentType contentType) // Set the "Content-Type" header.
        {
            switch (contentType)
            {
                case RequestContentType.x_www_form_urlencoded:
                    _request.ContentType = "application/x-www-form-urlencoded";
                    break;
                case RequestContentType.XML:
                    _request.ContentType = "text/xml";
                    break;
                case RequestContentType.JSON:
                    _request.ContentType = "application/json";
                    break;
                default:
                    _request.ContentType = "application/x-www-form-urlencoded";
                    break;
            }

            return this;
        }

        internal HttpRequestHeaderBuilder Cookies(CookieContainer container) // Set a cookie container.
        {
            _request.CookieContainer = container;
            return this;
        }

        internal HttpRequestHeaderBuilder AutoRedirect(bool allow) // Set AllowAutoRedirect
        {
            _request.AllowAutoRedirect = allow;
            return this;
        }

        internal HttpRequestHeaderBuilder Referer(string referer) // Set referer.
        {
            _request.Referer = referer;
            return this;
        }

        internal HttpRequestHeaderBuilder UserAgent(string agent) // Set referer.
        {
            _request.UserAgent = agent;
            return this;
        }

        internal HttpRequestHeaderBuilder Custom(string name, string value) // Set referer.
        {
            _request.Headers.Add(name, value);
            return this;
        }



        internal HttpRequestClient AddBody(IDictionary<string, string> values) => _client.AddBody(values);

        internal HttpRequestHeaderBuilder AddHeader() => _client.AddHeader();

        internal async Task<HttpRequestClient> SendAsAsync(MethodType method) => await _client.MethodBuilder.SendAsync(method);

    }

    private struct HttpRequestMethodBuilder
    {

        internal async Task<HttpRequestClient> SendAsync(MethodType method)
        {
            switch (method)
            {
                case MethodType.GET:
                    return await GetAsync().ConfigureAwait(false);
                case MethodType.POST:
                    return await PostAsync().ConfigureAwait(false);
                default:
                    return await GetAsync().ConfigureAwait(false);
            }
        }

        private async Task<HttpRequestClient> PostAsync() // Takes an object of WebRequestBodyBuilder (which just makes a string like "param1=value1&param2=value2&...."
        {
            byte[] buffer = Encoding.UTF8.GetBytes(_client._stringBuilder.ToString());

            using (var reqStrm = await _request.GetRequestStreamAsync().ConfigureAwait(false)) // Write the body in the request stream.
            {
                reqStrm.Write(buffer, 0, buffer.Length);
            }

            return _client;
        }

        private async Task<HttpRequestClient> GetAsync()
        {
            await _request.GetResponseAsync().ConfigureAwait(false);
            return _client;
        }
    }

    // Not sure why you're treating the body as QueryString, instead of object or plain string as a body could be attach in the url or the request body and both are totally different!
    // Consider serializations from (JSON, XML specifically)
    // you must have at least an overload of object or string to pass the serialized body. 
    internal HttpRequestClient AddBody(IDictionary<string, string> values)
    {
        if (values is null) { throw new ArgumentNullException($"Argument {nameof(values)} can't be null"); }

        var _stringBuilder = new StringBuilder();

        foreach (var pair in values)
        {
            _stringBuilder
                .Append(WebUtility.UrlEncode(pair.Key))
                .Append('=')
                .Append(WebUtility.UrlEncode(pair.Value))
                .Append("&");
        }

        _stringBuilder.Remove(_stringBuilder.ToString().Length - 1, 1); // remove the last &

        return this;
    }

    internal HttpRequestHeaderBuilder AddHeader() => HeaderBuilder;

    internal async Task<HttpRequestClient> SendAsAsync(MethodType method) => await MethodBuilder.SendAsync(method).ConfigureAwait(false);



}

Usage :

var body = new Dictionary<string, string>
{
    {"SomeHeader","SomeValue" }
};

var request = new HttpRequestClient("")
    .AddHeader()
    .ContentType(HttpRequestClient.RequestContentType.x_www_form_urlencoded)
    .Accepts("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8")
    .Referer(@"https://website.com/login")
    .AutoRedirect(false)
    .AddBody(body)
    .SendAsAsync(HttpRequestClient.MethodType.POST);

The SendAsAsync I made it as final call, so when you call it you can't add any header or body. The things that need to be add is to check the existing headers, and body, as you don't want to override the body twice ! you need only one body, so prevent that from happening. While the header is a dictionary, so the keys are unique, if any duplicates, will throw an error by default, so handle that as well.

I used Enum to avoid rewriting the same strings (like content type).

I'm sure there a lot of needed work on it, but as I said, it's just to give you some insights.

| improve this answer | |
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.