4

In my .NET Web APIv2 project, I've found myself marking the scope of all my injected services as Singleton.

I'm using the Simple Injector DI framework (not that this matters for my specific question).

Here's a simplified example:

public class SqlConnectionFactory : IDbConnectionFactory
{
    public async Task<SqlConnection> GetOpenSqlConnectionAsync()
    {
        var connection = new SqlConnection("connstring");

        await connection.OpenAsync();

        return connection;
    }
}


public class UserService : IUserService
{
    private IDbConnectionFactory DbConnectionFactory { get; set; }

    public UserService(IDbConnectionFactory dbConnectionFactory)
    {
        DbConnectionFactory = dbConnectionFactory;
    }

    public async Task AddAsync(AddUserDto data)
    {
        using (SqlConnection connection = await DbConnectionFactory.GetOpenSqlConnectionAsync())
        {
            // Create a glorious SqlCommand...
            await cmd.ExecuteNonQueryAsync();
        }
    }
}

Some of my other services include some cryptography classes, which contain no state whatsoever, and could easily be static classes.

Here's the Simple Inject code, just for completion:

        var container = new Container();

        container.Register<IDbConnectionFactory, SqlConnectionFactory>(Lifestyle.Singleton);
        container.Register<IUserService, UserService>(Lifestyle.Singleton);

Based on how I'm creating the database connection, it would appear to me that a Singleton is fine for my SqlConnectionFactory class, as well as my UserService class, but please correct me if I'm wrong.

This leads me to ask, when would you (or wouldn't you) use a Singleton scope for an injected service? And if you can use a Singleton, why wouldn't you? Isn't there a performance benefit of not having to instantiate a new instance for every web request thread, or entry (Transient) to a method?

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
contactmatt
  • 18,116
  • 40
  • 128
  • 186
  • 2
    The answer to this question is actually rather complicated, but I personally tend to make my complete object graph based on singleton components. Everything you wish to register as Scoped likely runtime data and this should not be created when the object graph is constructed. – Steven Sep 27 '16 at 20:00
  • Related: https://stackoverflow.com/a/32033648/264697 – Steven Sep 28 '16 at 07:37
  • Interesting question – ArieKanarie Nov 15 '16 at 08:54

1 Answers1

1

As a general rule, use Singleton lifetime for thread-safe classes. There's no reason to create new instances all the time if a single instance can do the job as well. You do, however, also need to avoid Captive Dependencies.

In case of the OP, it looks to me as though all classes are stateless (and thereby thread-safe), so Singleton lifetimes look good to me.

Lifetime mismatches can be difficult to spot when using a DI Container, but become much easier to detect when using Pure DI. That's one of many reasons I prefer Pure DI.

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736