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.

• Nkosi: 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. other than that you logic is sound but can also benefit from some defensive coding (null check on the callback)
• Carmax: Thank you @Nkosi that's very helpful. I've not delved into unit testing much but would like to code in preparation for it. Would I need to provide an interface / facade for the entire HttpContext.Current or could I just do it for the Application bit?
• Nkosi: Only for what is needed. No need to expose something you are not going to use. You can always refactor as requirements change
• JanErikGunnar: Consider using HttpContext.Current.Cache which gives more flexibility regarding timeouts and what not.
• Carmax: Coming back 4 year later only to find it is my own question :)

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);
}

• JanErikGunnar: This: return cachedData.HasValue ? cachedData.Value : CreateCache(cacheKey, getDataCallback); can be more easily expressed as: "return cachedData ?? CreateCache(cacheKey, getDataCallback);"
• Flater: @JanErikGunnar: IIRC, that would try to return a value of type int? (i.e. cachedData), not int (i.e. cachedData.Value). I initially used ?? and switched to using the ternary specifically because of that consideration.
• JanErikGunnar: No, the ?? operator "de-Nullable" the return value: docs.microsoft.com/en-us/dotnet/csharp/programming-guide/…
• user73941: @Flater: you can use ?? if you cast it the right way: return (int)(cachedData ?? CreateCache(cacheKey, getDataCallback));
• Flater: @HenrikHansen: Sure, but that's not particularly readable.

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.

• Carmax: Very useful examples, it made things so much clearer. Thank you very much. I even cheated and copied the caching abstraction wholesale.

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;

• paparazzo: Oh happy day 2 down votes without a comment. Come on what is the problem?
• Carmax: Although I found the other answers very helpful, yours is actually the most succinct answer to my question, so I upvoted it
• dfhwze: @paparazzo Same here, upvote. However, you could have addressed the subtle change you made to the original posted flow. I first thought you copy pasted the OP.