Basic API wrapper around a Restful service - follow-up

Question

This is a follow up from my previous post: Basic API wrapper around a Restful service

I'm writing a basic wrapper around a restful service that returns a list of Stores. Naturally, I want to make it completely testable (that's a major requirement!).

I have split out my TopmanClient class so that just deals with the rest client bits (using RestSharp).

I then inject the TopmanClient class into my TopmanRepository:

public class TopmanClient : ITopmanClient
{
 private readonly IRestClient _restClient;
 private const string BaseUrl = "https://public.Topman.com/";
 private const string AcceptTenant = "uk";
 private const string AcceptLanguage = "en-GB";
 public TopmanClient()
 {
 _restClient = new RestClient(BaseUrl);
 _restClient.AddDefaultHeader("Accept-Tenant", AcceptTenant);
 _restClient.AddDefaultHeader("Accept-Language", AcceptLanguage);
 }
 public IRestRequest Request(string url)
 {
 return new RestRequest(url);
 }
 public IRestResponse<T> Execute<T>(IRestRequest request) where T : new()
 {
 return _restClient.Execute<T>(request);
 }
}
public class TopmanRepository : ITopmanRepository<Store>
{
 private readonly ITopmanClient _topmanClient;
 public TopmanRepository(ITopmanClient topmanClient)
 {
 _topmanClient = topmanClient;
 }
 public List<Store> Get(string query)
 {
 var request = _topmanClient.Request("stores");
 request.RootElement = "Stores";
 request.AddQueryParameter("q", query);
 var response = _topmanClient.Execute<List<Store>>(request);
 return response.Data;
 }
}

Answer 1

As @t3chb0t says, the implementation is clean enough already.

However, if you wanted to go one step further, I would inject the implementation of IRestSharp into the TopmanClient, so that you can then test the behaviour of your default implementation when an API error occurs. For instance, if the API returns a 404 Not Found, how will your client behave? How about if a deserialisation error occurs?

For double bonus points, you could run the test once passing in a mock IRestClient; then run the same test again passing in a real RestClient - you now have a test of your client and an integration test for the API (although make sure you are pointing at a non-production API, or a test account, or a sandbox). Whether or not you include those in your automated builds or automated deployments, I'll leave up to you.

Answer 2

All of these strings should be moved to a configuration file.

private const string BaseUrl = "https://public.Topman.com/";
private const string AcceptTenant = "uk";
private const string AcceptLanguage = "en-GB";

Hardcoding them into your lib is going to make it very difficult to run this against a test environment. Also, if any of these values would need to change, you'd have to recompile & redeploy this code. Life's much simpler when we can just modify some values in a config and move on with our lives.