HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Generic MemoryCache class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
memorycachegenericclass

Problem

I want to have a caching class to cache different types. I want each type to be cached in a different MemoryCache but in a generic way.

Am I doing it right?

internal static class RecordsCache
{
    private static Dictionary cacheStore;
    static private CacheItemPolicy policy = null;

    static RecordsCache()
    {
        cacheStore = new Dictionary();

        ObjectCache activitiesCache = new MemoryCache(typeof(Activity).ToString());
        ObjectCache lettersCache = new MemoryCache(typeof(Letter).ToString());
        ObjectCache contactssCache = new MemoryCache(typeof(Contact).ToString());

        cacheStore.Add(typeof(Activity).ToString(), activitiesCache);
        cacheStore.Add(typeof(Letter).ToString(), lettersCache );
        cacheStore.Add(typeof(Contact).ToString(), contactssCache );

        policy = new CacheItemPolicy();
        policy.Priority = CacheItemPriority.Default;
        policy.AbsoluteExpiration = DateTimeOffset.Now.AddHours(12);
    }

    public static void Set(string userID, int year, List records)
    {
        var cache = cacheStore[typeof(T).ToString()];
        string key = userID + "-" + year.ToString();
        cache.Set(key, records, policy);
    }

    public static bool TryGet(string userID, int year, out List records)
    {
        var cache = cacheStore[typeof(T).ToString()];
        string key = userID + "-" + year.ToString();

        records = cache[key] as List;
        return records != null;
    }

    public static void Remove(string userID, int year)
    {
        var cache = cacheStore[typeof(T).ToString()];
        string key = userID + "-" + year.ToString();
        cache.Remove(key);
    }
}

Solution

-
Instead of having a cache

private static Dictionary cacheStore;


you could have one keyed of the types:

private static Dictionary cacheStore;


This means you don't need to call typeof(T).ToString() everywhere.

-
You duplicate the code of building the key in all 3 methods - it should be extracted into a private BuildKey(string userId, int year) method. This means if you need to change how the key is built you only need to touch one method rather than all of them.

-
I would provide a Register method which accepts a type for which to create a new cache rather than hard-coding it in.

-
The cache policy is also not configurable - which means you are stuck with it unless you want to change the code.

-
You really really should think hard about whether a static cache is the right way to go. It will give you trouble in unit testing and it will also make it impossible for multiple classes to have different cache with different policies for example.

Code Snippets

private static Dictionary<string, ObjectCache> cacheStore;
private static Dictionary<Type, ObjectCache> cacheStore;

Context

StackExchange Code Review Q#43111, answer score: 7

Revisions (0)

No revisions yet.