Extension method to get results of type T from a cache

Question

Are there issues of any kind with this extension method?

using System;
using System.Web;

namespace AccountingBackupWeb.CacheExtensions
{
    public static class Extensions
    {
        public static T Get<T>(this System.Web.Caching.Cache cache, string key, Func<T> f) where T : class
        {
            T results = HttpContext.Current.Cache[key] as T;

            if (results == null)
            {
                results = f();
                HttpContext.Current.Cache[key] = results;
            }

            return results;
        }
    }
}

Answer 1

Logically, it makes sense to me that you would want to tie a key to a Func. This method could then just take the string and then delegate the responsibility to decide how to fetch the value to another class. Also, a few nitpicky points about what is actually there. I consider using letters as variables poor practice since it gives no context as to how it will be used. I would rename "f" to something like "retrieve" or "fetch". Also, there is no guarantee that running "f" will actually give you a non null value. In that case, I doubt you would want to store null in your cache.

Answer 2

Without some more context on what your goals and/or concerns are its hard to say. This seem ok, although passing delegate functions is relatively expensive so if hyper-performance is required, you should refactor that. Aside from that, the only issue may be HttpContext.Current being null which would cause a NullReferenceException, although I assume this is not possible given your hosting scenario.

Answer 3

As JoeGeeky said HttpContext.Current might be null and it is better to ckeck it. But I also want to point out that from architectural point of view it is always better to incapsulate your cache host (HttpContext.Current in your case), so your framework cache getter method won't depend on Web classes.

You can introduce an abstract class (or interface) CacheHost and on runtime configure which actual subclass will be used as cache host. In most of your cases it could be HttpContext.Current, but if you run a separate thread and there you will use methods that relies on your cache, then you will be able to leverage your cache mechanism even if there is no HttpContext.Current in that thread.

Answer 4

I just have a little issue with the method name itself. It's not given by the name of a method called "Get" that it actually sets the cache value to the result of f if not already set, and then returns it. How 'bout GetOrInitializeTo, and then having a Get method that just gets?

Answer 5

In addition to what's already been said:

• the key parameter might be null
• the f parameter might be null
• The mechanism is highly inefficient if you actually want to cache null for a specific key

Answer 6

Since the as T operation will result in null if the value is actually not null but is not of type T, you might consider doing an extra check when results is null to determine whether the value in cache at that key is actually null. Example implementation shown below:

    public static T Get<T>(this System.Web.Caching.Cache cache, string key, Func<T> f) where T : class
    {
        object value = cache[key];

        T results = value as T;

        if (results == null)
        {
            if (value != null)
            {
                // log a warning, because there is a non-null value in cache with this key, but it is not of the specified type.
            }

            results = f();
            cache[key] = results;
        }

        return results;
    }

Essentially, if results is null but value is not null, then that means that there is already some value in cache with the specified key that is not of type T. I would consider that situation a potential coding error, so I would most likely want to log a warning or an error if that were to happen.

Alternatively, you could throw an exception. That would allow you to keep the details of your logging mechanism out of this utility method.

Answer 7

Use var when the right-hand side of a declaration makes its type obvious:

T results = HttpContext.Current.Cache[key] as T;

Should be

var results = HttpContext.Current.Cache[key] as T;

Here, the as T clause makes it very obvious that results is of type T. Using var means that changing T becomes a single operation, as opposed to changing it in two places.