1
\$\begingroup\$

For a project I am working on I had to query a database for a number of "child objects". Now in the mapping POJO for said child objects a reference to the parent is kept. That reference is available in the database as a foreign key constraint.

But if I retrieve a collection of all child elements they all map to vastly less (on the order of 1.5k:1) parent objects.
To prevent accessing the database for each and every parent object lookup I decided to cache the results of parent-object queries for the duration of child-object retrieval.

To accomplish this in a typesafe, but generic way I've written the following class:

public class LookupCache<T,P,R> {
    private final T delegate;

    private final Map<BiFunction<T, P, R>, Map<P, R>> cache = new HashMap<>();


    public static <T,P,R> LookupCache<T,P,R> forObject(T object) {
        return new LookupCache<>(object);
    }

    private LookupCache(T object) {
        this.delegate = object;
    }

    public R lookupOrCompute(BiFunction<T,P,R> compute, P param) {
        Map<P,R> cachedResults = cache.computeIfAbsent(compute, a -> new HashMap<>());
        return cachedResults.computeIfAbsent(param, p -> compute.apply(delegate, p));
    }
}

The usage is pretty simple:

try (ResultSet dbResult = connection.createStatement().executeQuery("SELECT id, parent_id, property FROM child;")) {
    final BiFunction<ParentDAO, Long, Parent> parentLookup = ParentDAO::getById;
    final LookupCache<ParentDAO, Long, Parent> lookupCache = LookupCache.forObject(new ParentDAOImpl());
    List<Child> result = new ArrayList<>();
    while (dbResult.next()) {
        Parent p = lookupCache.lookupOrCompute(parentLookup, dbResult.getLong("parent_id"));
        result.add(new Child(dbResult.getLong("id")
            , Property.fromDbState(result.getLong("property"))
            , p));
    }
    return result;
} // catch clause and default return for error

Note that usage is not up for review here, what I'm significantly more interested in is:

  • Is this Cache designed in a way that's intuitive to use?
  • Am I possibly overcomplicating things by passing the DAO to the Cache instead of capturing it in a lambda?
  • Anything else I might have missed?
\$\endgroup\$
2
  • \$\begingroup\$ These two sentences are contradictory: Note that usage is not up for review here and Is this Cache designed in a way that's intuitive to use?. \$\endgroup\$ – t3chb0t Jan 27 '17 at 5:03
  • \$\begingroup\$ I meant the usage example in the first sentence,, and the API in the second. You're welcome to fix the wording \$\endgroup\$ – Vogel612 Jan 27 '17 at 7:21
1
\$\begingroup\$

A loadable Cache, aka LoadingCache

Guava's LoadingCache is a type of caching implementation that knows how to retrieve a result from a source (e.g. database in your case) with a provided CacheLoader. Caffeine, a 'Google Guava inspired in-memory cache', provides similar functionality too. These can be something to consider instead of your bespoke HashMap-based implementation.

Initializing and using your 'loader'

LookupCache<ParentDAO, Long, Parent> cache = LookupCache.forObject(new ParentDAOImpl());
// ...
Parent p = cache.lookupOrCompute(ParentDAO::getById, dbResult.getLong("parent_id"));

I think these two lines illustrate the potential messiness using your implementation can lead to. It is not immediately clear how ParentDAOImpl gets its database connection, when there can be some reuse here.

Also, the BiFunction application should be part of the loader's specification, rather than a method argument, so that the usage of the cache (returning Parent objects based on the ID) is more concise and clearer.

I understand you want to open up your implementation to potential usages such as:

Parent p = cache.lookupOrCompute(ParentDAO::getByName, dbResult.getLong("parent_name"));

But if you really do have such a use case next to retrieval-by-IDs, then you might want to consider constructing secondary caches with their own key indices, instead of keying with a nested BiFunction application.

Using lambdas as map keys

I think the biggest potential problem is that it's not obvious, or even predictable, how lambdas' equalities conform to Map.get(Object). From this StackOverflow answer:

I think what you're trying to get to is: if two lambdas are converted to the same functional interface, are represented by the same behavior function, and have identical captured args, they're the same

Unfortunately this is both hard to do (for non-serializable lambdas, you can't get at all the components of that) and not enough (because two separately compiled files could convert the same lambda to the same functional interface type, and you wouldn't be able to tell.)

As such, cache.computeIfAbsent(compute, a -> new HashMap<>()) only works as per expectation if and only if you can ensure your BiFunction implementation passed here has a consistent equals() contract, and lambdas do not offer that out-of-the-box.

\$\endgroup\$
1
\$\begingroup\$

Cache semantic

A cache is something optional. You should be able to disable the cache by always returning "null" in the lookup method without worrying that your algorithm fails. It only should be slower. I don't think your implementation will pass this check. The cache should NOT retreive new data on its own.

Cache scope

A cache has a well defined lookup and object scope. Your implementation has a cached object lookup scope of "method" and a cached object usage scope that is somehow greater. Both should fall into the same scope to satisfy symmetry. Without knowing your UseCase-Layer you most of the time want to have all objects needed available in the UseCase. Refreshing your UseCase will often cause refreshing your cache because you want to process a UseCase on the latest data.

One other thing: Your Child-Objects may not the only Objects that have references on the Parent-Object. Those objects you would have to handle with seperate caches so you cannot take advantage of the already loaded instances. Consider caching on In thhe Business-Layer if you have no Persistence Mapping Tool (JPA). Otherwise you should make use of Entity-Caches.

Lambda expressions

My opinion on lambda expressions is ambivalent. Of course you can have very compact and technically expressive statements. But technically expressive statements may not be semantically expressive. Often they hide bad design. You can compress code that it looks good in one method that would normally be splitted into two methods. But the only reason you may have need for two methods was bad design.

I consider usage of lambda expression rarely: Listener-notifications, mapping JPA objects to Busines-Objects or massive parallel number calculations. I never use lambda expressions to make code "shorter".

Approach

Simplify the data retreiving process:

public class ChildService {

    public List<Child> getChildren() {

        List<Child> result = new ArrayList<>();

        try (ResultSet resultSet = connection.createStatement().executeQuery("")) {

            while (resultSet.next()) {
                result.add(new ChildProxy(resultSet.getLong("parent_id")));
            }

        } catch (SQLException e) {
            ...
        }

        return result;
    }

}

Passing the parent id to the constructor of the Child object and resolving the parent through the DAO:

public class Child {

    protected long parentId;

    public Child(long parentId) {
        this.parentId = parentId;
    }

    public Parent getParent() {
        return new ParentDaoImpl().getById(this.parentId);
    }

}

Derive a Caching enabled ChildProxy from the "Child" and intercept the getParent()-method to check for a cached Parent-Object. If you would have a JPA-Layer you may not need this because OR-Mapper often have their own caching mechanisms that are sufficient for most usecases.

public class ChildProxy extends Child {


    public ChildProxy(long parentId) {
        super(parentId);
    }

    Parent getParent() {

        Parent parent = Cache.lookup(Parent.class, this.parentId);

        if (parent == null) {
            parent = super.getParent();
            Cache.put(this.parentId, parent);
        }

        return parent;
    }

}

Formulating your UseCase:

public class UseCase {


    private final List<Child> children = new ArrayList<>();


    public void init() {
        ...
        this.children.addAll(new ChildService().getChildren());
        ...
    }


    public void reset() {
        Cache.clear();
        this.children.clear();
        init();
    }

    ...

}

Maybe you have to think a little bit more about the caching scope. It's not that easy because most developers tend to simply store expensive objects near their first occurance.

My advice is: NEVER cache at the first place. ALWAYS formulate your statements as they are NOT EXPENSIVE. After that you can go on with an optimization...

... that should be transparent to the existing implementation

... and has a well defined scope to match the use case requirements

\$\endgroup\$
2
  • \$\begingroup\$ if I had an O/R-Mapper I'd not have implemented this. It's nearly trivial to get any sufficiently useful O/R-M to cache lookups for me. What you're proposing is a complete reinvention of an O/R-M. It's basically not useful for me, because then I would've used an O/R-M in the first place. As such the only useful part of your answer to me is "Cache Semantics" and "Cache Scope". Especially of note is the fact that the cache lifetime and lookup lifetime is the method scope for my case. While it's theoretically possible to have those diverge it wouldn't make sense in usage (and doesn't happen here) \$\endgroup\$ – Vogel612 Jan 26 '17 at 22:13
  • \$\begingroup\$ I do not propose reinventing an OR mapper. That was never the point. I only say that method local caching prohibits method reusage in contexts that have their own definition of consistency. I do not see the possibility that the method is able to handle it properly so you should not load the method with that burden. Caching belongs to the use case layer or higher because only there you have the neccessary information to lock and cache to have a consistent view on your data in a collaborative system. \$\endgroup\$ – oopexpert Jan 27 '17 at 6:34

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.