Is it valid to expect/throw an exception using a custom caching system?

Question

Imagine some code like the following:

class Cache {
    private Map<String, String> values = new HashMap<String, String>();

    public String getFromCache(String key) {
        if (!values.containsKey(key)) {
            throw new NoEntryException();
        }
        return values.get(key);
    }

    public void setInCache(String key, String value) {
        this.values.put(key, value);
    }
}

class Foo {

    private Cache cache = new Cache();

    public String bla(String foo) {
        try {
            return cache.getFromCache(foo);
        } catch (NoEntryException ex) {
            String result = doSomeHeavyOperation(foo);
            cache.setInCache(foo, result);
        }
    }

    public String doSomeHeavyOperation(String something) {
        // heavy operation
        return something;
    }
}

Is this a valid use case for an exception? On one hand, it might be computationally slower to throw the exception, than doing a "cacheContains(String)" method or checking for null (although in that case, does null mean "contains no element" or "always null for doSomeHeavyOperation"?). On the other hand it does affect the control flow, but it doesn't seem to do so in a way that makes it less clear to me. I'd even say it looks cleaner to me than checking up front using a cacheContains(String).

The values in the cache are always the same, for the sake of simplicity (although you can expect a cache invalidation after 30 minutes).

Of course the cache used here is quite simple, but later on will be a wrapper around a more complicated system, like a redis integration.

Edit:

Here is some example code on how to do the accepted answer in Java (needs improvement with generics, multiple arguments, etc):

abstract class MissingResultCaller {
abstract String doCall(String argument);
}

class Cache {
private Map<String, String> values = new HashMap<String, String>();

public String getFromCache(String key, MissingResultCaller missingResultCaller) {
    if (!values.containsKey(key)) {
        String value = missingResultCaller.doCall(key);
        setInCache(key, value);
        return value;
    }
    return values.get(key);
}

public void setInCache(String key, String value) {
    this.values.put(key, value);
}
}

class Foo {

private Cache cache = new Cache();

public String bla(String foo) {
    return cache.getFromCache(foo, new MissingResultCaller() {
        @Override
        String doCall(String argument) {
            return doSomeHeavyOperation(argument);
        }
    });
}

public String doSomeHeavyOperation(String something) {
    System.out.println("Did heavy operation");
    return "heavy operation return";
}
}

Answer 1

You should prefer programmatic checks to exceptions whenever you can. An exception is exceptional, you should only throw them where a condition arises that you cannot correct. And please, don't use them for control flow.

It's fine to throw an exception in your cache GET, but ONLY if you also provide calling code with a way to check beforehand if the exception should be thrown. Have a look at most implementations of a filereader. It will throw an exception if you ask it to read beyond the end of a file, but it will also give you a means of determining if you have reached the end of said file.

Programmatic checks are clearer, easier to read and don't kill your performance. I cringe at the idea that you design a caching layer (intended to improve performance) but at the same time use exceptions in that layer for normal control flow.

If you don't want to return NULL, consider returning an intermediate object. You could return a CacheResult-object from your cache-calls which has a 'CacheStatus' and 'Result'-property. CacheStatus would have a value of either 'Hit' or 'Miss', and in the case of a 'Hit' you could find the cache object in the Result-property. This way you can work around the need for a 'null'-check if that bothers you.

Alternatively, you could pass a function to the cache method to allow the object to be created if it does not exist. Not sure if this can be done in all languages, C# would look a bit like this

private ResultType GetFromCacheOrCreate<ResultType>(string key, Func<ResultType> createFunction)
{
    var result = _myCache.Get<ResultType>(key);
    if(result == null)
        return createFunction();
    else
        return result;
}

private string doSomeHeavyOperation()
{
    //do something here
}

var myValue = GetFromCacheOrCreate<string>("mycachedValue", () => { return doSomeHeavyOperation(); });

Answer 2

Generally speaking an exception is called an exception because it should be an exception.

Throwing an exception will (as far as I know, correct me if I'm wrong) invalidate the whole command-pipeline and are therefore to be considered to be very slow. Exceptions will however give you the benefit of not having to perform any null-checks AND allow you to separate error-handling from actual code.

Now to come back to your specific problem: Since your cache is valid for 30 Minutes, performance really isn't an issue. The question you should be asking yourself therefore is:

Is the code readable? - Looks good to me.