1

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";
}
}
  • If you use Java 8, you can use lambdas the same as in C# (in @JDT's answer) instead of anonymous objects. Another - related - approach is to use lazy evaluation. See: blog.agiledeveloper.com/2015/06/lazy-in-java_24.html. It's an idiom not supported on language level in Java, but it can be implemented. Lazy objects are sort of mini-caches themselves, caching their own value - or evaluating it on demand and caching it from then on. Your cache could just store a bunch of Lazy objects. The fact that each of them knows how to evaluate itself could lead to better encapsulation, although YMMV. – Konrad Morawski Jul 27 '15 at 7:32
  • Actually, it seems to me I'm actually doing the lazy loading on the level of the Cache class right now. Right now it will store the results of the calls. I'd just need to structure the keys better (something like methodname.keyname instead of just keyname). – Kristof Jul 27 '15 at 7:41
  • Yes, laziness itself is the same concept, only executed on a different level. The difference with using a Lazy type is that the evaluator (or value supplier) is kept together - coupled - with the value container rather than being a separate entity. It may or may not be what you want, depending on the context – Konrad Morawski Jul 27 '15 at 7:49
4

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(); });
  • The reason why I'm doing this is to prevent a bunch of extra lines of code to do the checking for the cache hit, and otherwise doing the call to the someHeavyOperation, etc. An ideal scenario seems to me that you can actually somehow pass the concept of the call to the someHeavyOperation and its arguments to the caching layer, so that the caching layer will either do the call or return the value it already stored. But that's something I have no idea about whether it's even possible in Java :-) – Kristof Jul 27 '15 at 6:50
  • See my edited answer, don't know about Java but that's how I'd do it in C# – JDT Jul 27 '15 at 7:09
  • Actually, I was giving it some more thought myself, and I think I came to the same conclusion as you. It's possible to give some kind of object which the Cache can use to do the call on in case the result is missing/outdated. Thanks a lot for the suggestion though, this looks like a great answer for this usecase. – Kristof Jul 27 '15 at 7:16
  • +1 to "An exception is exceptional". C# and Java differ in many ways, but at least in this way they are very much the same. An exception should only occur if something happens that you didn't expect. A cache miss is definitely something to expect. – Neil Jul 27 '15 at 7:35
  • It's a nitpick, but your GetFromCacheOrCreate never actually caches the result createFunction returns – Konrad Morawski Jul 27 '15 at 9:59
0

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.

  • The 30 minutes is an arbitrary number. It could be 30 seconds as well, for example. The system should be able to handle a heavy load as well, so I don't think that "30 minutes" is something that should be focused upon. I have run into several opinions that an exception should not be used to control the flow, and this is a case where it does seem to do this exactly. It's not exactly an error being thrown in this case. – Kristof Jul 27 '15 at 6:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.