Is it ok to wrap a singleton?

I’m using SignalR with StackExchange Redis. What I gather from the
basic usage doc is the way to implement this is using a singleton. I’ve never used this patter before. My class looks like this:

public class RedisInstance
{
    public static readonly RedisInstance Instance = new RedisInstance();
    public static ConnectionMultiplexer Redis;

    public RedisInstance()
    {
        Redis = ConnectionMultiplexer.Connect("localhost");
    }
}

I’ve been using it like this

public class DashHub : Hub
{
    public void MapUser(int userId)
    {
        var user = new AppUser() {Id = userId, ConnectionIds = new List<string>() {Context.ConnectionId } };
        var db = RedisInstance.Redis.GetDatabase();

        var userJson = JsonConvert.SerializeObject(user);

        db.StringSet(userId.ToString(), userJson);
    }

But I was thinking this is rather constricting if I build this out and down the line want to use something other than Redis, so I was going to do this

public class DatabaseRepository
{
    IDatabase db;

    public DatabaseRepository()
    {
        this.db = RedisInstance.Redis.GetDatabase();
    }

    public void Insert(string key, string value)
    {
        db.StringSet(key, value);
    }
}

and then reference this in my other classes. Am I using these patterns appropriately here? Are there any issues with using a singleton in this way? This may seem straightforward but I’m just learning this and want to make sure.

I’ve not had much experience using redis so this code may not be 100% correct. But it would be better if you used dependency injection to manage your singleton. That way you aren’t having to code your class to be a singleton and if you want/need to change it at any point it will be very simple. eg

DI layer

builder.RegisterType<RedisInstance>().As<IRedisInstance>().SingleInstance();
builder.RegisterType<DatabaseRepository>().As<IDatabaseRepository>();

Redis Instance

public class RedisInstance : IRedisInstance
{
    public static ConnectionMultiplexer Redis;

    public RedisInstance()
    {
        Redis = ConnectionMultiplexer.Connect("localhost");
    }
}

DatabaseRepository

public class DatabaseRepository: IDatabaseRepository
{
   private readonly IRedisInstance db;

   public DatabaseRepository(IRedisInstance redisInstance)
   {
      this.db = redisInstance;
   }

   public void Insert(string key, string value)
   {
     this.db.Redis.StringSet(key, value);
   }
}

Usage

public class DashHub : Hub
{
  private readonly IDatabaseRepository repo;

  public DashHub(IDatabaseRepository repo)
  {
   this.repo = repo;
   }
    public void MapUser(int userId)
    {
        var db = this.repo.Insert("blah", "blah");
    }
}

Additional

I’m not sure why you would want your data access to be in singleton mode as you would want your connection to close and having a singleton will mean that multiple requests will be accessing the same instance. So you can now change your dependency injection to be like so:

builder.RegisterType<RedisInstance>().As<IRedisInstance>().InstancePerLifetimeScope();

When your code has a singleton, I would assume that anyone can use that singleton. Otherwise, it is not a singleton (a class that has by coincidence only one instance is not a singleton. A class that by design can only have one instance is a singleton).

No you say you “wrap a singleton”. The wrapper would again be a singleton. (You can of course have a class with many instances that each hold a reference to the same singleton, but that wouldn’t be a wrapper).

So you have a singleton, and a wrapper singleton which behind the scenes refers to the same singleton. I would be worried that being able to use the same singleton in two different ways would cause confusion.

Try something like (pseudo code):

public Interface IKeyValueRepository
{
    void Insert(string key, string value);
     //Put other methods here
}

//Redis Implementation
public RedisRepository : IKeyValueRepository
{
    private static ConnectionMultiplexer _redis;

    public RedisRepository ()
    {
         _redis = ConnectionMultiplexer.Connect("localhost");
    }

    public void Insert(string key, string value)
    {
        var db = _redis.GetDatabase();       
        db.StringSet(key, value);
    }
    //And so forth
}

Then you should be able to swap thing out pretty easily and mock the repository as well.

softwareengineering

Trả lời Hủy