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
    Commented Jan 30, 2018 at 13:59

1 Answer 1

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 and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.