1
\$\begingroup\$

I used to get a JSON file with some configuration settings from the file system, using this class:

public class ConfigurationFileService : IConfigurationService
{
    private readonly IConfigurationManager _configurationManager;

    public ConfigurationFileService(IConfigurationManager configurationManager)
    {
        _configurationManager = configurationManager;
    }

    public Configuration GetConfiguration(int agentId)
    {
        var path = _configurationManager.GetAppSetting("pathToConfigurationFile");

        var Configuration = JObject.Parse(File.ReadAllText(string.Format(path, agentId)));
        return Configuration.ToObject<Configuration>();
    }
}

Now I will be getting that from an API (it will return exactly the same JSON), so I need to change the implementation of the IConfigurationService, I have done this so far.

public class ConfigurationApiClient : IConfigurationService
{
    private readonly HttpClient _client;

    public ConfigurationApiClient()
    {
        _client = new HttpClient();
        _client.BaseAddress = new Uri(""); //TODO Add base url
        _client.DefaultRequestHeaders.Accept.Clear();
        _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    }

    public Configuration GetConfiguration(int agentId)
    {
        return GetConfigurationAsync(agentId).Result;
    }

    private async Task<Configuration> GetConfigurationAsync(int agentId)
    {
        try
        {
            var response = await _client.GetAsync(""); // TODO add the url
            response.EnsureSuccessStatusCode();
            var Configuration = await response.Content.ReadAsAsync<Configuration>();
            return Configuration;
        }
        catch(HttpRequestException ex)
        {
            throw new ConfigurationException(string.Format("Exception when getting  settings for agent: {0}", agentId), ex);
        }
    }

The interface is very simple:

public interface IConfigurationService
{
    Configuration GetConfiguration(int agentId);
}

I am not sure about a couple of things:

  1. The way I reuse the httpclient. I have seen that the recommendation is not to dispose it after each request, so I guess putting it in the constructor is a good option as the class itself will be injected.

  2. The usage of one public method with the signature of the interface, that blocks the result of the asynchronous method (given that's the correct way of getting the result asynchronously).

\$\endgroup\$
  • \$\begingroup\$ Would you be willing to change the return type of GetConfiguration to Task<Configuration>? \$\endgroup\$ – mjolka Nov 12 '15 at 13:39
  • \$\begingroup\$ @mjolka, not sure what you mean. Maybe skipping having two methods? \$\endgroup\$ – mitomed Nov 12 '15 at 13:49
  • \$\begingroup\$ I mean changing IConfigurationService to look like this: public interface IConfigurationService { Task<Configuration> GetConfigurationAsync(int agentId); } \$\endgroup\$ – mjolka Nov 12 '15 at 13:51
  • \$\begingroup\$ Oh I see @mjolka, well I thought it would be better to maintain the interface as the one accessing the file system returns Configuration \$\endgroup\$ – mitomed Nov 12 '15 at 14:07
1
\$\begingroup\$
public interface IAsyncConfigurationService : IConfigurationService{
    Task<Configuration> GetConfigurationAsync(int agentId);
}

public class ConfigurationApiClient : IAsyncConfigurationService
{
    //...
    public Configuration GetConfiguration(int agentId)
    {
        return GetConfigurationAsync(agentId).Result;
    }

    async Task<Configuration> IAsyncConfigurationService.GetConfigurationAsync(int agentId)
    {
        try
        {
            var response = await _client.GetAsync(""); // TODO add the url
            response.EnsureSuccessStatusCode();
            var Configuration = await response.Content.ReadAsAsync<Configuration>();
            return Configuration;
        }
        catch(HttpRequestException ex)
        {
            throw new ConfigurationException(string.Format("Exception when getting  settings for agent: {0}", agentId), ex);
        }
    }
}

With this, at least, you're able to try to do a cast to this interface and call the assynchronous version.

If you are able to change the interface just a little bit and still have only the synchronous version, you could add an event, for when the configuration is ready:

public interface IConfigurationService
{
    void LoadConfiguration(int agentId);
    event EventHandler<ConfigurationArgs> ConfigurationLoad;
}

public class ConfigurationArgs{
    public Configuration Config{get;set;}
}

public class ConfigurationApiClient : IConfigurationService
{
    //...
    public event EventHandler<ConfigurationArgs> ConfigurationLoad;

    private void OnConfigurationLoaded(Configuration config){
        var handler = ConfigurationLoad;
        if(handler != null){
            handler(this, new ConfigurationArgs{
                Config = config
            });
        }
    }

    public void LoadConfiguration(int agentId)
    {
        GetConfigurationAsync(agentId).ContinueWith(e => {
            OnConfigurationLoaded(e.Result);
        });
    }

    async Task<Configuration> GetConfigurationAsync(int agentId)
    {
        try
        {
            var response = await _client.GetAsync(""); // TODO add the url
            response.EnsureSuccessStatusCode();
            var Configuration = await response.Content.ReadAsAsync<Configuration>();
            return Configuration;
        }
        catch(HttpRequestException ex)
        {
            throw new ConfigurationException(string.Format("Exception when getting  settings for agent: {0}", agentId), ex);
        }
    }
}

The semantic now is that you can call LoadConfiguration any number of times (because I guess it could change), and have the result on the event handler.

\$\endgroup\$
  • \$\begingroup\$ the 'not really' synchronous version :p \$\endgroup\$ – Bruno Costa Nov 12 '15 at 15:54

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.