# ASP.Net caching manager

I have a Cache Helper Class.

using System;
using System.Web;

public static class CacheHelper
{
/// <summary>
/// Insert value into the cache using
/// appropriate name/value pairs
/// </summary>
/// <typeparam name="T">Type of cached item</typeparam>
/// <param name="o">Item to be cached</param>
/// <param name="key">Name of item</param>
public static void Add<T>(T o, string key)
{
// NOTE: Apply expiration parameters as you see fit.
// I typically pull from configuration file.

// In this example, I want an absolute
// timeout so changes will always be reflected
// at that time. Hence, the NoSlidingExpiration.
HttpContext.Current.Cache.Insert(
key,
o,
null,
System.Web.Caching.Cache.NoSlidingExpiration);
}

/// <summary>
/// Remove item from cache
/// </summary>
/// <param name="key">Name of cached item</param>
public static void Clear(string key)
{
HttpContext.Current.Cache.Remove(key);
}

/// <summary>
/// Check for item in cache
/// </summary>
/// <param name="key">Name of cached item</param>
/// <returns></returns>
public static bool Exists(string key)
{
return HttpContext.Current.Cache[key] != null;
}

/// <summary>
/// Retrieve cached item
/// </summary>
/// <typeparam name="T">Type of cached item</typeparam>
/// <param name="key">Name of cached item</param>
/// <param name="value">Cached value. Default(T) if
/// item doesn't exist.</param>
/// <returns>Cached item as type</returns>
public static bool Get<T>(string key, out T value)
{
try
{
if (!Exists(key))
{
value = default(T);
return false;
}

value =  (T) HttpContext.Current.Cache[key];
}
catch
{
value = default(T);
return false;
}

return true;
}
}


and usage:

string key = "EmployeeList";
List<Employee> employees;

if (!CacheHelper.Get(key, out employees))
{
employees = DataAccess.GetEmployeeList();
Message.Text =
}
else
{
Message.Text = "Employees pulled from cache.";
}


Do you see any improvement / issue?

/// <summary>
/// Insert value into the cache using
/// appropriate name/value pairs
/// </summary>
/// <typeparam name="T">Type of cached item</typeparam>
/// <param name="o">Item to be cached</param>
/// <param name="key">Name of item</param>
public static void Add<T>(T o, string key)


I think this signature would be more useful like this:

/// <summary>
/// Insert value into the cache using
/// appropriate name/value pairs.
/// </summary>
/// <typeparam name="T">Type of cached item (inferred from usage).</typeparam>
/// <param name="key">A string used to uniquely identify the added value.</param>
/// <param name="value">Item/value to be cached.</param>
public static void Add<T>(string key, T value)


/// <summary>
/// Remove item from cache
/// </summary>
/// <param name="key">Name of cached item</param>
public static void Clear(string key)
{
HttpContext.Current.Cache.Remove(key);
}


Clear is usually a parameterless method that clears a container's contents. This method should be named Remove.

/// <summary>
/// Check for item in cache
/// </summary>
/// <param name="key">Name of cached item</param>
/// <returns></returns>
public static bool Exists(string key)
{
return HttpContext.Current.Cache[key] != null;
}


"Check for item in cache" doesn't really say what's going on. I realize it's trivially inferred from the method's name, but "Checks for existence of specified key in cache." would be totally unambiguous.

public static bool Get<T>(string key, out T value)


I don't like the implementation of that method very much, there are too many exit points, some being redundant. Consider something like this:

public static bool TryGet<T>(string key, out T value)
{
bool result;

try
{
if (Exists(key))
{
value =  (T)HttpContext.Current.Cache[key];
result = true;
}
}
catch(InvalidCastException)
{
value = default(T);
}

return result;
}

• Thank you for very useful style guidelines. But I dont understand about using HttpContext.Current.Cache.Items. HttpContext.Current.Cache does not contain a definition for Items. If you mean HttpContext.Current.Items this is not the same. Items is per request, so its only available for that given user for that given HTTP request. Cache is stored in memory for a persistent period of time, and it not dependent on the specific user. So cache can be shared across multiple users across multiple requests, but Items is per user per request. – sDima May 13 '14 at 10:49
• I meant the indexer - Cache[n] should be the same as Cache.Items[n]. I'll read up on that, I don't use an HttpContext very often. – Mathieu Guindon May 13 '14 at 11:00
• Yes, I know it, but I can't use HttpContext.Current.Cache.Contains(key), because in HttpContext.Current.Cache has no definition of Contains() or something like this. – sDima May 13 '14 at 11:14
• Looks like I was looking at the wrong MSDN docs.. the indexer uses the Item property and indeed there's no Items property. Sorry for the mix-up, edited my answer. – Mathieu Guindon May 14 '14 at 16:06
try
{
if (!Exists(key))
{
value = default(T);
return false;
}

value =  (T) HttpContext.Current.Cache[key];
} catch {
value = default(T);
return false;
}

return true;


This contains some duplicate logic. We can rewrite it like this:

if (Exists(key))
{
value = (T) HttpContext.Current.Cache[key];
return true;
}

value = default(T);
return false;


• No negative condition check
• No duplication of the default(T)
• No expensive try-catch (all) block
• +1 I like this simple & neat implementation. However try/catch is only "expensive" if the stack gets unwinded, i.e. if an actual exception is thrown... and there's a possible InvalidCastException if the cached object can't be cast to T - granted it's a misuse, but still a possible execution path. The class can't assume its client knows what it's doing ;) – Mathieu Guindon May 12 '14 at 16:47
• Good point about the InvalidCastException. If you want to explicitly catch it then you can add it but it would give the impression that a key is not present and thus would hide the problem in its implementation. Since it is impossible in the first place to use multiple equal keys, I think throwing the exception is more appropriate (it also helps with debugging) – Jeroen Vannevel May 12 '14 at 16:55
• not necessarily: the method infers the type of T from the value parameter, which puts the entire responsibility of knowing the type of a cached item, on the caller, regardless of whether or not the specified key exists, hence my distrust for the [assumed-to-be-]safe cast. – Mathieu Guindon May 12 '14 at 17:04
• The alternative by catching the exception would be to return null so the user would have to explicitly call Exists before Get and make some weird flow to account for the possibility that the key might exist but he's trying to retrieve it with the different type (which doesn't show up any different from not having the key in the first place). Since there shouldn't be any surprise as to what type a key is, I don't think it adds any value to hide the exception (but it will show up as an immediate problem if you used the wrong type). – Jeroen Vannevel May 12 '14 at 17:10
• Damn, blinded by the trees again - you're right, it's best to just let the InvalidCastException bubble up, it's the most meaningful result a caller can get (vs a default(T) value and a false result for a key that does exist). – Mathieu Guindon May 12 '14 at 17:13

I would suggest one more solution how to make usage a bit easier. You can add this method into CacheHelper

public static T GetOrAdd<T>(string key, Func<T> getter)
{
T value;
if (!TryGet<T>(key, out value))
{
value = getter();
}

return value;
}


In that case usage can be a bit simplier, but in that case you won't know is item existed in cache or not

string key = "EmployeeList";
List<Employee> employees = CacheHelper.GetOrAdd(key, () =>  DataAccess.GetEmployeeList());
// or even like this if method matches signature

• Yeah, with TryGet<T> it looks better! – Sergey Litvinov May 14 '14 at 8:48