0
votes

I have a call with a static method in my web application. This method uses a singleton instance of a different class. This singleton instance is basically an HttpClient to make some api requests.

Currently I don't have any mechanism to automate multi-user testing on this. Are there any consequences of this approach? I know a usual static method is thread safe if we are not using static variables inside it. But I am not sure how it behaves in case of singleton.

public static class API
{
    private static System.Net.Http.HttpClient httpClient;

    public static System.Net.Http.HttpClient Instance
    {
        get
        {
            return httpClient ?? (httpClient = new System.Net.Http.HttpClient());
        }
    }
}

public static async Task<string> GetData(string id)
{
    HttpResponseMessage response = await 
    API.Instance.GetAsync(string.Format(requestURL, id));
    response.EnsureSuccessStatusCode();

    // return URI of the created resource.
    return await response.Content.ReadAsStringAsync();
}
1

1 Answers

1
votes

In order to avoid threading issues, you need to at least implement the singleton in a thread-safe manner. This article by Jon Skeet describes some ways how a singleton instance can be created in a thread-safe way. In addition, you should make sure that the methods of the singleton can either handle parallel requests or use lock to synchronize the calls.

The problem with using the Singleton in the static method is also one of object-oriented design. Having a singleton and using it in lots of places of your application is convenient, but has its downsides:

  • A fellow developer cannot see on the first look that your methods rely on a singleton instance. It is a hidden dependency.
  • Also, you cannot easily change the instance in cases where you need a different behavior, e.g. in tests.

So I'd propose to think about whether the singleton is really necessary or whether you can inject it as a parameter into the methods where it is needed:

public static async Task<string> GetData(API api, string id)
{
    HttpResponseMessage response = await 
    api.Instance.GetAsync(string.Format(requestURL, id));
    response.EnsureSuccessStatusCode();

    // return URI of the created resource.
    return await response.Content.ReadAsStringAsync();
}

You could change the method to be an extension method that you can call like:

var result = await API.Instance.GetData("123");

You just have to add a this to the signature:

public static async Task<string> GetData(this API api, string id)
{
  // ...
}