C# fetch connection details inside a constructor

Question

I have my DB class defined as below:

public class DbAdapterService : DbAdapterService
{
      private readonly AppSettings _settings;
      public DbAdapterService(IOptions<AppSettings> settings)
      {
           _settings = settings?.Value;
           DbConnectionStringBuilder builder = new DbConnectionStringBuilder();
           builder.ConnectionString = _settings.ConnectionString;           
      }
}

In the above class I am fetching my connection string from my appsettings.json and it works fine.

Now we need to fetch the connecting strings username and password from another method defined in our class. This method fetches these details from our stored vault. Example as below:

public class CredentialsService : ICredentialsService 
       {   
         public Credentials GetDetails()
         {
            //return credentials
         }
       }

My questions is can I call this method in my DbAdapterService constructor above or if there is a better way to handle this.

Thanks

--Updated--

public class DbAdapterService : DbAdapterService
{
      private readonly AppSettings _settings;
       public ICredentialsService  _credentialsService;
       private bool isInitialized = false;
      
      public DbAdapterService(IOptions<AppSettings> settings, ICredentialsService  credentialsService)
      {
           _settings = settings?.Value;
           
           _credentialsService = credentialsService;
            if (!isInitialized)
            {
                Initialize(_credentialsService);
            }
           
           DbConnectionStringBuilder builder = new DbConnectionStringBuilder();
           builder.ConnectionString = _settings.ConnectionString;           
      }
      
       public void Initialize(ICredentialsService  credentialsService)
        {
            if (isInitialized)
                return;
            else
            {
               //credentialsService.GetDetails();
                isInitialized = true;
            }          
            
        }
}

Generally a constructor should not be reaching out to other services. If the DbAdapterService needs to get connection details from elsewhere, it should have an Initialize (or InitializeAsync) method that you can invoke, and that method can reach out to API's or databases or wherever in order to get the values it needs. — mason, Jul 16 '21 at 18:20
@GertArnold True, I never said it wasn't. I'm afraid I don't understand the point of your comment. Can you please clarify why you said that? — mason, Jul 16 '21 at 20:33
@GertArnold Reaching out to another service (API or a database for example) is an inherently dangerous operation, and often needs to be done asynchronously. That sort of logic doesn't belong in a constructor body. See [Meligy's answer here](https://stackoverflow.com/a/4605517/1139830). Since that answer was written, async code has become far more prolific, and there's not really a pleasant way of calling async code from a constructor. There's a line in the question here "`This method fetches these details from our stored vault." - we haven't seen the details of that, but it might be async — mason, Jul 16 '21 at 20:49
@mason can you share an example of how can I have Initialize method along with my constructor to fetch these details — Brenda, Jul 19 '21 at 13:19
It wouldn't be in the constructor. That's the point: constructors aren't a good place for that sort of thing. Instead, you'd put that logic into a method, and you can invoke that method to go and grab the details from whatever external source you need. — mason, Jul 19 '21 at 13:39
Thanks I understand not putting that logic inside constructor and having that logic instead to a different method. But where do I need to invoke that method from? Is this similar to what below answer has been posted. Sorry just trying to understand this better. — Brenda, Jul 19 '21 at 14:42
You have several choices. You could make it something that has to be explicitly done before you invoke the other functions of the object (though that's error prone). Or you could put the invoke it from the other functions in the method that need it, perhaps behind a boolean to make sure it only gets initialized once, like `private void EnsureInitialized() { if (_initialized) return; else { /* initialization logic here */ _initialized = true; } }`. The idea there is to make your object easy to use so that other code doesn't have to know to initialize it. — mason, Jul 19 '21 at 14:47
@mason I have updated my code above as per my understanding. Let me know if this is not what you mean by. Thanks — Brenda, Jul 19 '21 at 16:39
You're still invoking the initialization logic from your constructor. That defeats the purpose. Instead, either invoke it from the logic that uses your DbAdapterService, or invoke it in the methods of DbAdapterService that depend on being initialized. — mason, Jul 19 '21 at 16:42
Ok got that part but my issue is that the service which I am calling fetches the user name and password for the connection string. Once I have these user name and password I plan to replace them in my builder.ConnectionString that I am defining at the constructor on the line: builder.ConnectionString = _settings.ConnectionString; With your suggestion that means I need to move this connection string to every method which is going to use it. Am I correct? — Brenda, Jul 19 '21 at 16:50
No, not exactly. I've posted an answer which conveys what I'm talking about, though I replaced the _initialized boolean with just checking the connection string, since if it's null we can assume it hasn't been initialized. — mason, Jul 19 '21 at 17:09

score 1 · Answer 1 · answered Jul 16 '21 at 18:23

you can try this

public class DbAdapterService : DbAdapterService
{
      private readonly AppSettings _settings;
      private readonly ICredentialsService _credentialsService ;

      public DbAdapterService(
     IOptions<AppSettings> settings, 
      ICredentialsService credentialsService )
      {
          _credentialsService= credentialsService ;

           _settings = settings?.Value;
           DbConnectionStringBuilder builder = new DbConnectionStringBuilder();
           builder.ConnectionString = _settings.ConnectionString;           
      }
}

after this you can call any method from the credentialService

or a little shorter if you only need credentials

      private readonly AppSettings _settings;
     private readonly Credentials _credentials;

      public DbAdapterService(
     IOptions<AppSettings> settings, 
      ICredentialsService credentialsService )
      {
        _credentials= credentialsService.GetDetails();

           _settings = settings?.Value;
           DbConnectionStringBuilder builder = new DbConnectionStringBuilder();
           builder.ConnectionString = _settings.ConnectionString;           
      }

So its ok to inject the service and then call on of its method in the constructor — Brenda, Jul 16 '21 at 18:38
@Brenda Yes, you can call only one method if you only need credentials. — Serge, Jul 16 '21 at 18:53

mason · Accepted Answer · 2021-09-15T02:13:33.997

You seem to want to initialize the connection string in the constructor. If you're reaching out to some external component (file system, database, or API for example) to retrieve a value, that's possibly going to be an async operation. There's no reliable way of calling an async method in a constructor.

So, what can we do? Well, there's no rule saying we must do it in the constructor. The constructor was a convenient place, because it ensures that by the time you invoke any other methods, the initialization will have taken place. But there are other patterns to accomplish this. Here's one:

public class DbAdapterService : IDbAdapterService
{
    string _connectionString;

    readonly AppSettings _settings;
    readonly ICredentialsService _credentialsService;
    
   public DbAdapterService(IOptions<AppSettings> settings,
                           ICredentialsService  credentialsService)
   {
       _settings = settings.Value;         
       _credentialsService = credentialsService;         
   }

   async Task EnsureInitializedAsync()
   {
       if (!string.IsNullOrEmpty(_connectionString))
       {
           //no need to reinitialize
           //unless the credentials might change during the lifecycle of this object
           return; 
       }

       var credentials = await _credentialsService.GetDetailsAsync();
       var builder = new DbConnectionStringBuilder(_settings.ConnectionString);
       builder.Username = credentials.Username;
       builder.Password = credentials.Password;
       _connectionString = builder.ConnectionString;         
   }

   public async Task DoSomethingAsync()
   {
       await EnsureInitializedAsync();

       //now you can use _connectionString here
   }
}

The key part is remembering to invoke EnsureInitializedAsync() in any method that needs to make use of the connection string. But at least code that consumed DbAdapterService won't have to know whether to initialize the connection string or not.

While this pattern isn't as necessary for non-async code, it's great for operations that might become async in the future, and the pattern makes more sense if the details of the connection might actually change at runtime, but your objects are constructed when you configure IoC container.

Thanks this makes more sense. – Brenda Jul 20 '21 at 13:10 — Brenda, Jul 20 '21 at 13:10

C# fetch connection details inside a constructor

2 Answers2