CacheManager - Dependency injection

Question

I create a cache manager to manage a system of cache exchangeable by another implementation.

I use the CacheManager class throughout my program. If I want to change the implementation I simply add a provider and I add it in my dependency container

Is it correct to do this like that?

ICache

namespace Caching
{
 public interface ICache
 {
 object Get(string key);
 void Set(string key, object value);
 }
}

ICacheManager

namespace Caching
{
 public static class CacheManager
 {
 private static ICache cache;
 private static readonly object syncRoot = new object();
 private static ICache Cache
 {
 get
 {
 lock (syncRoot)
 {
 if (cache == null)
 {
 cache = Container.Instance.Resolve<ICache>();
 }
 }
 return cache;
 }
 }
 public static object Get(string key)
 {
 return Cache.Get(key);
 }
 public static void Set(string key, object value)
 {
 Cache.Set(key, value);
 }
 }
}

MemoryCacheProvider

using System.Runtime.Caching;
namespace Caching.Providers
{
 public class MemoryCacheProvider : ICache
 {
 public object Get(string key)
 {
 return MemoryCache.Default.Get(key);
 }
 public void Set(string key, object value)
 {
 MemoryCache.Default.Add(key, value, DateTime.Now.AddMinutes(5));
 }
 }
}

Main

static void Main(string[] args)
{
 Container.Instance.RegisterType<ICache, MemoryCacheProvider>();
}

• 200_success: You have asked thrice for this question to be deleted. Stack Exchange policy forbids deletion of posts that have upvoted answers. Furthermore, self-vandalism is against the rules. I have rolled back and locked this post.

Answer 1

You are not doing dependency injection but using another pattern which is called service locator which isn't by the way so cool becasue classes using it depend on the Container:

cache = Container.Instance.Resolve<ICache>();

The CacheManager resolves an ICache from the Container. None of your classes should even know that the Container exists (but the one that configures it).

You should remove the CacheManager entirely and instead correctly inject the ICache to classes that require it.

class ThisClassRequiresCache
{
 public ThisClassRequiresCache(ICache cache) { .. }
}

Your dependency injection framework should handle the injection.

---

As far as the private Cache properety is concerned you should add another cache == null check to prevent unnecessary locks after the cache has already been created:

get
{
 if (cache == null)
 {
 lock (syncRoot)
 {
 if (cache == null)
 {
 cache = Container.Instance.Resolve<ICache>();
 }
 }
 }
 return cache;
}