3
\$\begingroup\$

i have created one generic implementation for api call from all over my application.

 public class HttpClientService<TResult> : IDisposable where TResult : class
{
    private HttpClient client = new HttpClient();
    public HttpClientService(string baseUrl)
    {
        client.BaseAddress = new Uri(baseUrl);
        client.DefaultRequestHeaders.Accept.Clear();
        client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        client.Timeout = new TimeSpan(0, 30, 0);
    }
    public TResult GetAPI(string url, Dictionary<string,string> urlParameters = null, Dictionary<string, string> headers = null)
    {
        string parameters = string.Empty;

        if (urlParameters != null)
        {
            parameters = BuildURLParametersString(urlParameters);
            url = string.IsNullOrEmpty(parameters) ? url : url + parameters;
        }

        if (headers != null)
        {
            AddHeaders(headers);
        }

        var response = Task.Run(() => client.GetAsync(url)).Result;
        var apiResult = response.Content.ReadAsStringAsync().Result;
        var result = JsonConvert.DeserializeObject<TResult>(apiResult);
        return result;
    }
    public TResult PostAPI(string url, Dictionary<string, string> formParameters = null, string jsonString = "", Dictionary<string, string> headers = null)
    {
        HttpContent contentPost = null;
        if (headers != null)
        {
            AddHeaders(headers);
        }
        if (formParameters != null)
        {
            var formContent = new FormUrlEncodedContent(formParameters);
            contentPost = formContent;
        }
        else if (!string.IsNullOrEmpty(jsonString))
        {
            var content = new StringContent(jsonString, Encoding.UTF8, "application/json");
            contentPost = content;
        }
        var response = Task.Run(() => client.PostAsync(url, contentPost)).Result;
        var apiResult = response.Content.ReadAsStringAsync().Result;
        var result = JsonConvert.DeserializeObject<TResult>(apiResult);
        return result;
    }
    private String BuildURLParametersString(Dictionary<string, string> parameters)
    {
        UriBuilder uriBuilder = new UriBuilder();
        var query = System.Web.HttpUtility.ParseQueryString(uriBuilder.Query);
        foreach (var urlParameter in parameters)
        {
            query[urlParameter.Key] = urlParameter.Value;
        }
        uriBuilder.Query = query.ToString();
        return uriBuilder.Query;
    }
    private void AddHeaders(Dictionary<string, string> headers)
    {
        foreach (var header in headers)
        {
            if (!string.IsNullOrEmpty(header.Value))
            {
                client.DefaultRequestHeaders.Add(header.Key, header.Value);
            }
        }
    }
    public void Dispose()
    {
        client.Dispose();
    }
}

and from my application used like below

using (var clientService = new HttpClientService<GeoCodeResponse>("https://maps.googleapis.com"))
{
         var param = new Dictionary<string, string>();
         param.Add("latlng", $"{latitude},{longitude}");
         param.Add("result_type", "locality");
         param.Add("key", GoogleApiKey);

         var apiResult = clientService.GetAPI("maps/api/geocode/json", param);   
}

please suggest me what i am implemented is currect or any better improvement for code any changes.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ I suggest using a single immutable HttpClient instance per API service. One potential problem with your approach: if you generate too many HttpClientService<T> in a set amount of time, you will run out of sockets, and thus encounter timeouts. Take a look at aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong \$\endgroup\$
    – Brad M
    Jan 30 '18 at 13:59
3
\$\begingroup\$

A few points that come to mind:

  1. A lot of your code closely follows the underlying API of the HTTP client instance. It would be better to have a function that generates your client with its default settings (the ones you're setting in the constructor), then just use the appropriate methods on the HttpClient.

  2. There arent many (any?) good use cases for calling Result on a task. You should make your method async and await the task instead.

  3. Your use of Task.Run is redundant. You're wrapping one task inside another.

  4. What happens if there is an error? Incorrect URI, no network etc...

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