1

I have a repository class that has a constructor with string parameter argument. It is a connection string. I created an interface for it and I'm using Unity in my UI project.

My question is, how do I set this up the 'right' way so that Unity will know how to properly construct my class in order to inject it when instantiating my controller?

I currently 'worked around' this by using a parameterless constructor for my repository but feel like this is a cop out.

Here is my repository constructor I want to use...

    public CobraLettersRepository(string dbConnectionString)
    {
        _connString = dbConnectionString ?? throw new ArgumentNullException(dbConnectionString);

        dbConnection = new SqlConnection(_connString);
        dbConnection.Open();
    }

I created ICobraLettersRepository and want to inject it into my controller constructor.

    public CobraLetterController(ICobraLetterRepository cobraLetterRepository)
    {
        if (cobraLetterRepository == null)
            throw new ArgumentNullException(cobraLetterRepository);

        _cobraLetterRepo = cobraLetterRepository;
    }

When I try this, the code compiles but I get runtime errors whenever I attempt to navigate to a part of my app where those controller methods are called.

Kenneth K.
  • 2,987
  • 1
  • 23
  • 30
  • 1
    "I get runtime errors" Which ones? Please be more specific on your problem. From my perspective your code is okay, assuming `CobraLettersRepository` implements `ICobraLetterRepository`. Apart from this: you surely want `ArgumentNullException(nameof(dbConnectionString))`. – MakePeaceGreatAgain Jan 28 '19 at 13:51
  • 2
    You can supply a `func` that returns an instance of an object to the service e.g. `services.AddScoped((IServiceProvider) => new SQLLogger(connectionString));` – NotFound Jan 28 '19 at 13:54
  • 4
    Opening the connection in a constructor is a major bug. Connections should only be created and opened right before they are used and disposed right after that. The only thing a long-lived connection can do is accumulate locks and cause blocking. There's *NO* penalty from repeatedly opening/closing connnections, in fact it *improves* performance through connection pooling. – Panagiotis Kanavos Jan 28 '19 at 13:54

2 Answers2

2

I would say to encapsulate the Connection String inside a configuration object and put the get of the connection string inside the constructor of that configuration object. Then just register your configuration object as other class.

public class ProjectConfiguration
{
    public string ConnectionString { get; set; }

    public ProjectConfiguration()
    {
        ConnectionString = //Get the configuration from , i.e: ConfigurationManager
    }

}

And then, inject it :

public CobraLettersRepository(ProjectConfiguration configuration)
{
    if(configuration == null){
        throw new ArgumentNullException(configuration);
    }
    _connString = configuration.ConnectionString;

    dbConnection = new SqlConnection(_connString);
    dbConnection.Open();
}

With that you can always isolate the config from the DI container and leave the injection as simple as you can. Also, you can get the config from your config files (one per environment).

EDIT: Also check your repository constructor logic, maybe you don't want to Open the connection on the constructor. Is better to make a using statement on your class methods like :

using(SqlConnection connection = new SqlConnection(_injectedConfiguration.ConnectionString)){
    connection.Open();
    //Make your stuff here
}

With the using you ensure that the connection will be correctly disposed.

Note that with the configuration object approach you can just store the configuration object in a private readonly variable and use it whatever you need.

gatsby
  • 1,148
  • 11
  • 12
  • Ahh.. Good point. Opening a db connection probably isn't appropriate in a constructor. The solution I'm working on has many repos that do this. Open in constructor, implement IDisposable, close and dispose of connection in Dispose method. – douglas.kirschman Jan 28 '19 at 15:48
0

During registration use an InjectionConstructor to pass the connection string name. You need to remove all constructors except one that expects a string argument.

container.RegisterType<ICobraLetterRepository,CobraLettersRepository>(new InjectionConstructor("connectionstringname"));

Please note that it will lead to problems somewhere/sometime. You need to change the logic to create the database connection (you may refer to Breaking SOLID Principles in multiple implementation of an Interface or Resolve named registration dependency in Unity with runtime parameter).

daryal
  • 14,643
  • 4
  • 38
  • 54