Caching an integer - distinguishing no value from zero

Question

I would like to store an integer value in the application cache.

The GetOrSet method should always return an int, and never be null. If there is no cached value, then the getDataCallback function will provide the integer.

But I need the cache check to return a nullable int in order to distinguish no value from a 0 integer value.

I am looking for any faults in the code below, or possible improvements.

It probably looks very simple to others, but I find working with nullable ints quite confusing.

public int GetOrSet(string cacheKey, Func<int> getDataCallback)
{
    int? data = (int?)HttpContext.Current.Application[cacheKey]; // this should return null if there is no value stored in the cache
    if (! data.HasValue)
    {
        int intData = getDataCallback();
        HttpContext.Current.Application[cacheKey] = intData;
        return intData;
    }
    else
    {
        return data.Value;
    }
}

---

Update

Big thanks to both @Nkosi and @Flater for their help.

I marked @Flater's answer as correct because I had to chose one only. But I really hated having to do this as both answers were equally helpful and I used both their suggestions in the end. I would encourage anybody reading this to check them both out.

Answer 1

Your current method is slightly violating SRP, it's both checking is a cache exists, and creating one if needed. I'd prefer to separate the two:

public int GetData(string cacheKey, Func<int> getDataCallback)
{
    int? cachedData = (int?)HttpContext.Current.Application[cacheKey];

    return cachedData.HasValue
                        ? cachedData.Value
                        : CreateCache(cacheKey, getDataCallback);
}

private int CreateCache(string cacheKey, Func<int> getDataCallback)
{
    int intData = getDataCallback();
    HttpContext.Current.Application[cacheKey] = intData;
    return intData;
}

The first method only checks the existing cache and, if missing, calls the second method, which will create a new cache.

• The code could be refactored to slightly shorten it, but that would be at the cost of readability.
• I renamed GetOrSet to GetData for clarity's sake. The method exists to give you the data, regardless of whether it's cached or created on the fly. Pedantically, you'd need to name it Get_or_SetAndThenGet for it to be an accurate description. I prefer GetData, it sticks to the important facts and remains concise.
• Nkosi is also correct in the comments that you should ideally abstract this snippet away from using HttpContext, but that's not necessary for the question at hand.

---

Note that you could also stop using int? if you find it confusing:

public int GetData(string cacheKey, Func<int> getDataCallback)
{
    object cachedData = HttpContext.Current.Application[cacheKey];

    return cachedData != null
                        ? (int) cachedData
                        : CreateCache(cacheKey, getDataCallback);
}

Answer 2

Assuming the above code is not itself an abstraction, tightly coupling your code to HttpContext would make it not unit testable as HttpContext.Current is null outside of IIS.

Consider encapsulating that code behind an abstraction.

public interface IApplicationCache {
    object this[string cacheKey] { get; set}
}

public class ApplicationCacheImplementation {
    public object this[string cacheKey] { 
        get {
            return HttpContext.Current.Application[cacheKey];
        }
        set {
            HttpContext.Current.Application[cacheKey] = value;
        }
    }
}

whose implementation will be injected explicitly into the dependent class.

Other than that you logic is sound but can also benefit from some defensive coding (null checks on the provided arguments) and separation of concerns.

public class MyClass {
    private readonly IApplicationCache applicationCache;

    public MyClass(IApplicationCache applicationCache) {
        this.applicationCache = applicationCache;
    }

    public int GetOrSet(string cacheKey, Func<int> valueFactory) {
        if (string.IsNullOrWhiteSpace(cacheKey)) throw new ArgumentException("cacheKey");
        if (valueFactory == null) throw new ArgumentException("valueFactory");

        return (int)(get(cacheKey) ?? set(cacheKey, valueFactory));
    }

    private object get(string cacheKey) {
        return applicationCache[cacheKey];
    }

    private int set(string cacheKey, Func<int> valueFactory) {
        int value = valueFactory();
        applicationCache[cacheKey] = value;
        return value;
    }       
}

Note the naming changes to help make the code a little more readable.

Answer 3

More code than needed

int? data = (int?)HttpContext.Current.Application[cacheKey]; 
if (! data.HasValue)
{
    data = getDataCallback();
    HttpContext.Current.Application[cacheKey] = data;
}
return data.Value;