This is the second part in a series of articles in which I attempt to answer the question How much better would I be as a developer now, if I could have listened to the thought processes of other developers when they were writing applications?
Introduction
I remember, as a young developer, being in awe of people who could sit down and code, seemingly without any effort. Systems would seem to flow out of their fingers, effortlessly crafted, elegant, and refined. It felt like I was witnessing Michelangelo in the Sistine Chapel or Mozart sitting down in front of a fresh stave. Of course, with experience, I now know that what I was seeing was developers doing what developers do. Some were doing it well, and really understood the craft of development, while others were producing work that was less elegant and less well written. Over the years, I have been privileged enough to learn from some amazing developers but I keep coming back to the same basic question, time after time, namely…
How much better would I be as a developer now, if I could have listened to the thought processes of other developers when they were writing applications?
In this series of articles, I’m going to take you through what I’m thinking while I develop an application. The code that accompanies the article will be written as a “warts and all” development so you can see how I take something from the initial requirements phase through to something I would be happy for others to use. This means that the articles will show every mistake I make and the shortcuts I take while I’m fleshing ideas out. I’m not going to claim I’m a great developer here, but I am competent and experienced enough that this should help people who are new to the field get over their awe a lot earlier, and gain confidence in themselves.
Setting the Scene
This article is the follow up to Part 1 where we introduced the basic code that we are going to build on. In this article, I am going to add the actual GET calls as well as refactoring I want to put in place to support future HTTP operations.
Source Code
The code for this article can be downloaded from https://github.com/ohanlon/goldlight.xlcr/tree/article2.
Starting With a Refactoring Exercise
While I was writing the original GetRequest
method, I had a thought at the back of my mind; that the functionality we were building at that point was not just going to be used for calling the GET
methods. With that in mind, the first thing I want to do is to refactor the GetRequest
class and introduce a base class to inherit from. This class is going to be called Request
.
While I am writing this class, I am going to turn my attention to the intended use of the Execute
method. This method is intended to perform the actual operation, whether it's a POST
, a GET
or whatever. The operations themselves are asynchronous, so I want to consider making this method asynchronous friendly. If I were to call GetAsync
, for instance, this method returns a Task<HttpResponseMessage>
, so I get the hint that I can either manage its lifetime using Task.Run
semantics or using async
with await
. If I were going to batch up the HTTP calls and needed to wait for them all to complete, then I would use the Task
itself with WaitAll
. I don't need to do that here though, so I am going to use the extremely convenient async
/await
option, which means I need to make the method signature return Task
.
While I am making changes to the method signature in my mind, I am also considering that the current version of the code returns an object. As each of the asynchronous method calls returns Task<HttpResponseMessage>
, I think that would be a suitable return type from the new Execute
method. Something I have to remember, I have written a number of unit tests that call the Execute
method, so those tests will need some minor refactoring to support the change to the signature.
My first pass at this new class looks like this:
public abstract class Request
{
public Task<HttpResponseMessage> Execute(string endpoint)
{
return Execute(new Endpoint(endpoint, QueryString));
}
public QueryString QueryString { get; set; }
protected abstract Task<HttpResponseMessage> Execute(Endpoint endpoint);
}
I chose to make this class abstract
because I want to force classes that inherit from it to have to implement the abstract
method. What this means is, any class that inherits from our base class does not have to repeat the code where the Endpoint
instance is created. It's a minor thing, but it does mean that our Execute
method behaves in a consistent manner no matter where it's called from. You may notice that I have added a QueryString
property to our base class. I added this to complete the ability to transform the endpoint
.
When I was writing the initial GetRequest
implementation, I stopped short of actually performing the HTTP operation. I am now ready to address this deficiency, starting with the use of the HttpClient
class provided by Microsoft. I just need to make two small additions to the Request
class. First, I'm going to add a protected
HttpClient
property that all of the inheriting classes will have access to.
protected HttpClient Client { get; }
Next, I am going to provide a constructor that accepts an HttpClient
as a parameter. I choose to do this as a constructor because it means that each implementation class will be forced to supply this. If the inheriting classes fail to populate this, I want to force an exception to make sure that users of this class will be unable to complete instantiation, which prevents them from being able to try to issue an HTTP request.
public Request(HttpClient client)
{
Client = client ?? throw new ArgumentNullException(nameof(client));
}
I am now ready to change my GetRequest
class so that it performs the actual GET operation. Using inheritance, this class is transformed into the very lean:
public class GetRequest : Request
{
public GetRequest(HttpClient client) : base(client) { }
protected async override Task<HttpResponseMessage> Execute(Endpoint endpoint)
=> await Client.GetAsync(endpoint.Address);
}
Testing Redux
The changes here mean that the build will fail to complete because I have unit tests that use the default constructor, so they do not pass in a client.
In the GetRequest.Execute
method, I am calling GetAsync
so if my tests call this method, there would appear to be an assumption that the test is going to execute an actual HTTP call. While I could do this, there is a problem in that this makes the tests fragile and prone to breaking because I may not control the endpoint. I could write my own set of APIs and fire up a web server to perform the tests, but that is just going to slow the testing down as well.
I solve the problem of using an HTTP operation by tricking the HttpClient
a little. For many reasons, HttpClient
is not suitable for mocking. Fortunately, I do know that an HttpClient
instance can accept an instance of an HttpMessageHandler
. It is this instance that ultimately makes all of the HTTP calls possible through a protected
method called SendAsync
. What I need to be able to do is replace the HttpMessageHandler
class with my own implementation. Fortunately for me, I recently uploaded a project here on CodeProject that allows me to do just this. In my tests, I can now do things like this:
public async Task GivenCallToExecuteGetRequest_WhenEndpointIsSet_ThenNonNullResponseIsReturned()
{
FakeHttpMessageHandler fakeHttpMessageHandler = new FakeHttpMessageHandler();
HttpClient httpClient = new HttpClient(fakeHttpMessageHandler);
GetRequest getRequest = new GetRequest(httpClient);
Assert.NotNull(await getRequest.Execute("http://www.google.com"));
}
As I build the code out, I will demonstrate more advanced ways of using FakeHttpMessageHandler
to craft the responses we are expecting.
There are More to Requests Than Just Query Strings and Endpoints
A feature of web APIs is that they allow developers to supply information via request headers. These key/value pairs are used for things such as setting user agents for web browsers, authorization headers, timestamps and pretty much any other piece of information development teams and web frameworks can find useful. I need to support the ability to add request headers to an HttpClient
instance, and I am going to provide this in a class called RequestHeaders
.
It may not seem a lot but I had to spend a lot of time thinking about the name of this class. Did I want it to be called RequestHeader
or RequestHeaders
, after all I did name QueryString
as a singular rather than pluralising it to QueryStrings
. The reason I debated this issue is because, ostensibly they are similar classes. They both internally maintain a dictionary of string
keys with string
values and they both apply some form of transformation operation inside the class itself. The reason I opted for the name I did came down to the intent of the class. The QueryString
class is aimed at transforming the QueryString
, which is a singular entity. The RequestHeaders
class is intended to add values into the HttpClient DefaultRequestHeaders
collection, so it is acting on multiple items. Basically, I have named the class according to its intent rather than to any internal storage mechanism.
As I was contemplating the code I intended to write, I had to ask myself a question. Is it acceptable for a request header to have an empty value? My assumption was that this was a valid thing to do but it is dangerous to make assumptions so I needed to research whether or not this was the case. My first stop was the documentation for DefaultRequestHeaders to see what it had to say on the subject. While reading this, I didn't see anything that indicated I couldn't add empty string
s so these would be among the first tests I write. While I'm thinking about the "unhappy paths" through the code, I have had to ask myself whether or not I need to validate that the value is unique. This comes because the http header collection allows us to add multiple values for a header. Again, there is no constraint that values have to be unique.
As it is possible to have multiple values against the same header, I decided that I will be using a dictionary
where the values will be a list of string
s. What this means is, when I am adding a header, I first need to see if the header is present or not. If it's not present, I'm going to create it with an empty list. Then I add the value to the list.
My first pass of the code looks like this:
public class RequestHeaders
{
private readonly Dictionary<string, List<string>> _headers =
new Dictionary<string, List<string>>();
public void Add(string key, string value)
{
if (string.IsNullOrWhiteSpace(key))
throw new ArgumentException("You must supply the header name.");
if (!_headers.ContainsKey(key))
{
_headers[key] = new List<string>();
}
_headers[key].Add(value);
}
}
I now need a means to apply the headers to the HttpClient
. I am going to add the following method to RequestHeaders
, which will apply the transformation.
public void Apply(HttpClient httpClient)
{
foreach (KeyValuePair<string, List<string>> header in _headers)
{
httpClient.DefaultRequestHeaders.Add(header.Key, header.Value);
}
}
While I was writing tests for this code, I mistakenly called Apply
twice (I say mistakenly but it was a good job that I did because I missed the fact it would be duplicating the values in the header). In order to prevent this becoming a potential issue, I have to alter the Apply
method slightly. In order to satisfy the following test.
[Fact]
public void GivenRequestHeader_WhenCallingApplyMoreThanOnce_ThenOnlyOneValueIsPresent()
{
HttpClient httpClient = new HttpClient();
RequestHeaders requestHeader = new RequestHeaders();
requestHeader.Add("test", "value");
requestHeader.Apply(httpClient);
requestHeader.Apply(httpClient);
IEnumerable<string> headers = httpClient.DefaultRequestHeaders.GetValues("test");
Assert.Single(headers);
}
I need to make one small modification to my Apply
method to clear the headers before I add them back in.
public void Apply(HttpClient httpClient)
{
httpClient.DefaultRequestHeaders.Clear();
foreach (KeyValuePair<string, List<string>> header in _headers)
{
httpClient.DefaultRequestHeaders.Add(header.Key, header.Value);
}
}
Back to the Request
I have the ability to support request headers. I have a Request
class. All I need to do now is make a couple of amendments to the Request
class to support this. I am going to add a RequestHeaders
property to the class, and then use this property in the Execute
method. This is what the Request
class looks like now:
using System;
using System.Net.Http;
using System.Threading.Tasks;
namespace Goldlight.Xlcr.Core
{
public abstract class Request
{
public Request(HttpClient client)
{
Client = client ?? throw new ArgumentNullException(nameof(client));
}
public Task<HttpResponseMessage> Execute(string endpoint)
{
RequestHeaders?.Apply(Client);
return Execute(new Endpoint(endpoint, QueryString));
}
public QueryString QueryString { get; set; }
public RequestHeaders RequestHeaders { get; set; }
protected HttpClient Client { get; }
protected abstract Task<HttpResponseMessage> Execute(Endpoint endpoint);
}
}
By the way, because it is possible that I may not always be passing in a RequestHeaders
into this, I check for null
before calling Apply
. I could have wrapped the Apply
portion in an if (RequestHeaders != null)
block but I like the use of the ?
null operator here as it does not alter the flow of reading for me. An aside here; as I knew that I intended to call an additional method inside Execute
, this was the reason I didn't use expression syntax to coalesce the method down to:
public Task<HttpResponseMessage> Execute(string endpoint) =>
Execute(new Endpoint(endpoint, QueryString));
Conclusion
So, there we have it. We have created an MVP for our GetRequest
with what is relatively simple code. I haven't tried to be clever. I haven't tried to take shortcuts and there's still a lot that can be done to improve on this code. In the next article, I will tackle what I need to do to call this from a user interface. As a spoiler, I still haven't decided what technology I am going to use for the UI; I will debate my decision in the next installment so that you can see how I make my choice.
I hope you like the series. There's a lot to do with it and I am having an absolute blast with it.
History
- 13th May, 2021: Initial version