Imran Ahmad Shahid
Imran Ahmad Shahid

Reputation: 822

Simple Injector Registration of Instance after calling GetInstance method

I have a scenario that i have a Client of webAPi that need base url in it's constructor and An configuration manager class that reads configuration from web.config.

interface IConfigManager
{
   string baseurl {get;}
}
class ConfigManager:IConfigManager
{
 public string baseurl => system.configuration.ConfigruationManager.AppSettings["URL"];
}

and i have a client class that calls the web api

interface IApiClient
{
 List<Products> GetProducts(string addresssufix);
}

public class ApiClient:IApiClient
{
public ApiClient(string baseUrl)
{
//----
}
 List<Products> GetProducts(string addresssufix)
{
//....
}
}

so i require Url in APiClient

in simple injector while registration of components.

 container.Register<IConfigManager, ConfigManager>();
var config= container.GetInstance<IConfigManager>();

container.Register<IApiClient<(()=> new ApiClient(config.baseurl));

but it said i can't register after calling GetInstance

Upvotes: 1

Views: 2264

Answers (2)

Steven
Steven

Reputation: 172646

Simple Injector blocks any calls to Register after the first call to GetInstance to force strict registration between registration and resolve. This design prevents strange, hard to debug, and hard to verify behaviour as explained in more detail here in the documentation.

But just as you want to separate the registration phase from the phase where you start resolving from the container, you should do the same when reading configuration values. Configuration values should only be loaded at application start-up, before or during the time of the registration phase. Delaying the reading of configuration values causes applications to become brittle and forces you to go through the complete application to find out whether or not the application is configured correctly, while this can easily be prevented by loading the configuration at start-up (and thus letting the application fail fast).

This means, that in your situation it doesn't make much sense to have an IConfigManager abstraction, since it's only goal is to delay the loading of base URL from the app settings, while again, this should be done directly at start-up (and should preferably fail in case that value is missing or malformed).

With that in mind, I would like to propose the following improvement and simplification to your design:

var container = new Container();

string baseUrl = System.Configuration.ConfigruationManager.AppSettings["URL"];

if (string.IsNullOrEmpty(baseUrl))
    throw new ConfigurationErrorsException("appSettings/URL is missing.");

container.RegisterSingleton<IApiClient>(new ApiClient(baseUrl));

See how the configuration is read directly at startup and checked immediately whether the value exists. After that the baseUrl is used directly in the ApiClient constructor. Also note that ApiClient is registered as Singleton. I assume here that ApiClient is stateless and immutable (which is a good practice).

Note that you did the right thing by letting your ApiClient depend on the string baseUrl configuration value, instead of injecting the IConfigManager. Using ConfigManager as an abstraction in application code is typically problematic. Such configuration abstraction will typically grow indefinitely during the lifetime of the application, since every time a new configuration value is added to the configuration, this abstraction (and its implementation and possible fake implementations) need to be updated. Consumers of that abstraction will typically only depend on one or a few configuration values, but never on all. This is an indication of a Interface Segregation Principle violation. Problem with that is that it becomes harder to test consumers of that interface, because you typically want to be sure that they use the correct configuration values. Another problem is that from the definition of such a consumer (its type name and its constructor with required dependencies) it becomes impossible to see which configuration values are actually required.

All these problems go away completely when you let consumers depend directly on the configuration value they require. But again, this even removes the need to have this IConfigManager abstraction in the first place.

Do note that although register-resolve-register is not permitted, you would be able to do the following instead:

container.Register<IConfigManager, ConfigManager>();

container.Register<IApiClient>(() =>
    new ApiClient(container.GetInstance<IConfigManager>().baseurl));

What is going on here is that GetInstance<IConfigManager> is called as part of the delegate for IApiClient. This will work because in that case the GetInstance<IConfigManager>() is called while resolving the IApiClient and thus after the registration process. In other words, resolving IConfigManager is delayed.

Big warning about this though: This practice is typically not advised. As explained before, when it comes to configuration values, we don't want to load them lazily. But even in other cases, we typically don't want to do this, because this construct blinds Simple Injector's verification and diagnostic system. Since Simple Injector uses statically available information (such as constructor arguments) to analyse your object graphs, such dynamic call will disallow Simple Injector to find common problems such as Lifestyle Mismatches. In other words, this construct should only be used in rare cases were you're sure that misconfigurations won't occur.

Upvotes: 3

hyankov
hyankov

Reputation: 4130

Pass the dependent object to ApiClient, instead of just a property of it. If that object has too many properties in which ApiClient is not interested, do interface segregation.

container.Register<IConfigManager, ConfigManager>();
container.Register<IApiClient, ApiClient>();

public ApiClient(IConfigManager configManager)
{
   this.baseurl = configManager.baseurl;
}

container.GetInstance<ApiClient>();

Upvotes: 1

Related Questions