5
\$\begingroup\$

I wrote a HttpClient instantiable class that will be used as reference to simplify API calls in other methods inside other classes, so the user could call it in a simple way when using my project as reference. I made both synchronous and asynchronous methods, as it was requested for me to made like this.

The main class to build the client is built in:

public class Client : IDisposable
{
    private static HttpClient _client;

    public Client(bool payment)
    {
        var baseUrl = payment ? "https://payment.apiaddress.com/" : "https://api.apiaddress.com/";

        _client = new HttpClient {BaseAddress = new Uri(baseUrl)};
        _client.DefaultRequestHeaders.Accept.Clear();
        _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        _client.DefaultRequestHeaders.Add("X-API-KEY", Config.GetToken());
    }

    public void Dispose() => _client.Dispose();

    public HttpResponseMessage Request(Methods method, string url, object data)
    {
        switch (method)
        {
            case Methods.GET: return _client.GetAsync(url).Result;
            case Methods.POST: return _client.PostAsJsonAsync(url, data).Result;
            case Methods.PUT: return _client.PutAsJsonAsync(url, data).Result;
            case Methods.DELETE: return _client.DeleteAsync(url).Result;
            default: return _client.GetAsync(url).Result;
        }
    }

    public string Get(string url) => 
        Request(Methods.GET, url, null).Content.ReadAsStringAsync().Result;

    public string Post(string url, object data) => 
        Request(Methods.POST, url, data).Content.ReadAsStringAsync().Result;

    public string Put(string url, object data) => 
        Request(Methods.PUT, url, data).Content.ReadAsStringAsync().Result;

    public string Delete(string url) => 
        Request(Methods.PUT, url, null).Content.ReadAsStringAsync().Result;

    public async Task<HttpResponseMessage> RequestAsync(Methods method, string url, object data)
    {
        switch (method)
        {
            case Methods.GET: return await _client.GetAsync(url).ConfigureAwait(false);
            case Methods.POST: return await _client.PostAsJsonAsync(url, data).ConfigureAwait(false);
            case Methods.PUT: return await _client.PutAsJsonAsync(url, data).ConfigureAwait(false);
            case Methods.DELETE: return await _client.DeleteAsync(url).ConfigureAwait(false);
            default: return await _client.GetAsync(url).ConfigureAwait(false);
        }
    }

    public Task<HttpResponseMessage> GetAsync(string url) => 
        RequestAsync(Methods.GET, url, null);

    public Task<HttpResponseMessage> PostAsync(string url, object data) => 
        RequestAsync(Methods.POST, url, data);

    public Task<HttpResponseMessage> PutAsync(string url, object data) => 
        RequestAsync(Methods.PUT, url, data);

    public Task<HttpResponseMessage> DeleteAsync(string url) => 
        RequestAsync(Methods.DELETE, url, null);
}

It's used on other classes like this one, to start a new checkout for custom objects based on specific models, which are made by a third part:

public class Payment
{
    private static readonly Client _client = new Client(true);

    public static async Task<string> CreditAsync(object data)
    {
        var response = await _client.PostAsync("Payment", data);
        var content = await response.Content.ReadAsStringAsync();
        return content;
    }

    public static string Credit(object data)
    {
        var response = _client.Post("Payment", data);
        return response;
    }
}

The new Client(true) value is to provide the both expected address for the API, as there will be moments that in production environment it will use the api.etc and other that will use the payment.etc, also to use the provided relative path inside the method content as well.

So the usage could be simplified just as:

public void Credit() //OR: public async Task CreditAsync()
{
    var transaction = new Transaction<Credit>
    {
        PaymentMethod = new PaymentMethod {Code = "1"},
        Application = "Tests",
        Vendor = "Felipe",
        Customer = new Customer
        {
            //Customer data goes here...
        },
        Products = new List<TransactionProduct>
        {
            //Product data goes here...
        }
    };

    var test = Payment.Credit(transaction); //OR: await Payment.CreditAsync(transaction);
    Console.WriteLine(teste);
}

It works fine and provide the expected responses, that I'll do a better treatment later as required, but I would really appreciate any different and more experienced views to know if this could be considered a good approach for this need and/or if there are ways to improve it, as the idea is to release it as a NuGet/SDK for the final project.

\$\endgroup\$
6
\$\begingroup\$

Dispose Strategy

It seems your class doesn't even need to implement IDisposable. Should dispose HttpClient?


Single Responsibility

I would extract configuration management from the implementation of Client.

public Client(string baseUrl)
{
    _baseUrl = baseUrl ?? throw new ArgumentNullException(nameof(baseUrl));
    // ..
}

Add a factory for generating the client instance. This one uses hardcoded uri's, but you could also read from a config file or database.

public static class ClientFactory
{
    public static Client Create(bool payment)
    {
        var baseUrl = payment 
            ? "https://payment.apiaddress.com/" : "https://api.apiaddress.com/";
        return new Client(baseUrl);
    }
}

Method Design

I have my doubts about having *..1..* (junction) method interactions. I would refactor this to 1..1 (simple) or 1..*..1 (fork).

Original *..1..*:

 public HttpResponseMessage Request(Methods method, string url, object data)
    {
        switch (method)
        {
            case Methods.GET: return _client.GetAsync(url).Result;
            case Methods.POST: return _client.PostAsJsonAsync(url, data).Result;
            case Methods.PUT: return _client.PutAsJsonAsync(url, data).Result;
            case Methods.DELETE: return _client.DeleteAsync(url).Result;
            default: return _client.GetAsync(url).Result;
        }
    }

    public string Get(string url) => 
        Request(Methods.GET, url, null).Content.ReadAsStringAsync().Result;

    // others ..

Refactored to 1..1:

public string Get(string url) => 
    _client.GetAsync(url).Result.Content.ReadAsStringAsync().Result;

public string Post(string url, object data) => 
    _client.PostAsJsonAsync(url, data).Result.Content.ReadAsStringAsync().Result;

public string Put(string url, object data) => 
    _client.PutAsJsonAsync(url, data).Result.Content.ReadAsStringAsync().Result;

public string Delete(string url) => 
    _client.DeleteAsync(url).Result.Content.ReadAsStringAsync().Result;

And if you really must have wrapper function 1..*..1:

 public HttpResponseMessage Request(Methods method, string url, object data)
    {
        switch (method)
        {
            case Methods.GET: return Get(url);
            case Methods.POST: return Post(url, data);
            case Methods.PUT: return Put(url, data);
            case Methods.DELETE: return Delete(url);
            default: return _client.GetAsync(url).Result;
        }
    }
\$\endgroup\$
4
\$\begingroup\$

I think, you should be concerned about this behavior:

public class Client : IDisposable {
  private static HttpClient _client;

  public Client(bool payment)
  {
    var baseUrl = payment ? "https://payment.apiaddress.com/" : "https://api.apiaddress.com/";

    _client = new HttpClient {BaseAddress = new Uri(baseUrl)};
    _client.DefaultRequestHeaders.Accept.Clear();
    _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    _client.DefaultRequestHeaders.Add("X-API-KEY", Config.GetToken());
  }

  public Uri BaseAddress => _client?.BaseAddress; // Added by HH
  ....

Test case

  // 
  Client client1 = new Client(true);
  Console.WriteLine(client1.BaseAddress);
  Console.WriteLine();
  Client client2 = new Client(false);
  Console.WriteLine(client1.BaseAddress);
  Console.WriteLine(client2.BaseAddress);

Output:

https://payment.apiaddress.com/

https://api.apiaddress.com/
https://api.apiaddress.com/

As you see the inner static HttpClient _client becomes a new instance when client2 is called and therefore client1 also changes base address which may result in unexpected behavior.

So as I see it, you can't use this concept with two different base addresses.

A solution could be to provide what could be called a "Dualton" in a fashion like:

public class Client : IDisposable
{
  private HttpClient _client;

  private Client(string baseUrl)
  {
    //var baseUrl = payment ? "https://payment.apiaddress.com/" : "https://api.apiaddress.com/";

    _client = new HttpClient { BaseAddress = new Uri(baseUrl) };
    _client.DefaultRequestHeaders.Accept.Clear();
    _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    _client.DefaultRequestHeaders.Add("X-API-KEY", Config.GetToken());
  }

  static Client paymentClient;
  static Client normalClient;

  public static Client Create(bool payment)
  {
    if (payment)
    {
      paymentClient = paymentClient ?? new Client("https://payment.apiaddress.com/");
      return paymentClient;
    }

    normalClient = normalClient ?? new Client("https://api.apiaddress.com/");
    return normalClient;
  }

Someone would probably called that an anti pattern?

A more general solution could be to have a static Dictionary<string, Client> clients and the matching public static Client Create(string baseUrl) {}.

\$\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.