Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I generally prefer to work with EF by specifying exactly the data to fetch from the DB with regards to Navigation Properties/Lookups and the like, so we wrote this cache system to limit the number of calls to the database.

public class DBCache<TContext, TKey, T> : ICache<TKey, T> where T : class, IDbPk<T>, IAutoIncludeEntity<T>, new()
    where TContext : DbContext, IQueryableRepository, IDisposable, new()
{
    protected Dictionary<TKey, T> _Items;

    /// <summary>
    /// To assist with derivations of this system.
    /// </summary>
    protected DBCache()
    {

    }

    /// <summary>
    /// Generate a cache of entries specified by selector.
    /// Incldue Nav Properties specified in IAutoIncludeEntity
    /// </summary>
    /// <param name="selector"></param>
    public DBCache(Func<T, TKey> selector)
    {
        using (var db = new TContext())
        {
            _Items = db.GetDictionary(selector);
        }
    }

    /// <summary>
    /// Generate a cache of entries specified by selector.
    /// Include Nav Properties specified in IAutoIncludeEntity
    /// </summary>
    /// <param name="context"></param>
    /// <param name="selector"></param>
    public DBCache(TContext context, Func<T, TKey> selector)
    {
        _Items = context.GetDictionary(selector);
    }

    /// <summary>
    /// Generate a cache of entries
    /// </summary>
    /// <param name="context"></param>
    /// <param name="selector"></param>
    /// <param name="includes">Include these navigation properties</param>
    public DBCache(TContext context, Func<T, TKey> selector, params Expression<Func<T, object>>[] includes)
    {
        _Items = context.GetDictionary(selector, includes);
    }

    /// <summary>
    /// Get an item from the cache.
    /// If it doesnt exist, try to get it from the store and if successful, add it to the cache.
    /// </summary>
    /// <param name="id"></param>
    /// <returns></returns>
    public T GetItem(TKey id)
    {
        if (_Items.ContainsKey(id))
            return _Items[id];
        using (var db = new TContext())
        {
            var contact = db.GetSingle<T>(new object[]{id});
            //miss on cache - so add it
            if (contact != null)
                _Items.Add(id, contact);
            return contact;
        }
    }

    /// <summary>
    /// Remove an item from the cache
    /// </summary>
    /// <param name="id"></param>
    public void Remove(TKey id)
    {
        _Items.Remove(id);
    }

    /// <summary>
    /// The Value collection (ICollection)
    /// </summary>
    public Dictionary<TKey, T>.ValueCollection Values { get { return _Items.Values; } }
}

Effectively I need to decorate Database first Entity Framework entities like so:

    /// <summary>
/// An interface for a class to provide default Navigation Property includes
/// </summary>
/// <typeparam name="T">Type of Entity</typeparam>
public interface IAutoIncludeEntity<T> where T : class
{
    Expression<Func<T, object>>[] Includes();
}

/// <summary>
/// An iterface for a class to provide the Primary Key Value
/// </summary>
public interface IDbPk<T>
{
    /// <summary>
    /// The Primary Key of this Entity
    /// </summary>
    object PrimaryKeyValue { get; }

    /// <summary>
    /// Expression to compare an instance via Primary Key
    /// </summary>
    /// <returns></returns>
    Expression<Func<T, bool>> PkComparer();
}

Was the rationale behind this system OK, as I primarily work with Detached Entities in EF. One of the driving factors was to speed up performance in my application (mainly by reducing lag). I understand that I need an efficient way to determine if the cache is stale, but in our scenario we can simply refresh after an elapsed time.

share|improve this question

2 Answers 2

Instead of reinventing the wheel, you could perhaps use the SharpRepository library for EntityFramework (NuGet SharpRepository.EntityFramework). This already has quite a few caching providers already that are interchangable. The con is that the DbContext is not very easily accessible through this library and so you would have to do some hacky magic if you want to use stored procedures.

On the upside, the cache is completely transparent and you won't even know it's there (Unlike your intrusive interface), and SharpRepository also comes with popular Ioc adapters out-of-the-box.

I appreciate this is not a very in-depth answer other than "use this, my google fu is amazing", so if this gets downvoted or similar I can understand :)

share|improve this answer
    
I actually like answer Dan, mostly because you describe there being quite a few caching providers out of the box. Despite generics being so damned cool (few love them more than me and my obfuscation-by-generic-abstraction), I think it's more important to consider the differences, pros, cons, etc between cache types. –  Smudge202 Aug 20 at 21:15
    
I prefer generics in cases like this because it means less boilerplate. At the end of the day it comes down to which tool is best for the job. SharpRepository comes with support for Memcached, AppFabric, Azure and Redis (!!) out of the box. Pretty neat. –  Dan Pantry Aug 20 at 21:18
    
I prefer generics for everything. I'd tie my shoes in the morning with generics if there was a way (hmmmmm). But for performance sensitive operations, there is no one hit wonder that solves all. Abstract through generics, for sure. Makes testing that much simpler. But concrete implementations don't need to be generic, especially for case by case optimising. –  Smudge202 Aug 20 at 21:25

You state in the OP that you designed this caching mechanism to limit the number of database calls. In my opinion, what you've actually done is provide some syntax candy that doesn't necessarily make a great deal of difference.

I'll start by saying that I'm assuming the intended use case of your caching mechanism is in areas of your application that have been diagnosed as a bottle neck and resulting performance testing showed improved sufficiently improved performance to justify the increased memory usage. Right?

I suspect there are a couple types of performance issues, based on my experiences. The first is the straight up "we perform this query very often" issue.

To be honest, if you have any kind of unit of work that ensures reuse of dbcontext/connection, these actually aren't as expensive as you might think at first glance. If you're using a SQL server backend for example, the database engine is rather remarkable at both query optimisation and query caching using special notification subscriptions built into the engine to catch staleness.

You may have a slow network however, in which case I recommend a caching technique. Depending on volatility and size of data, that could be anything from storing the data in singleton thread safe cache using SQL dependencies for updates, to your approach of populating during a unit of work. However, its unlikely to find a single caching mechanism which is optimal for all cases, even in smaller projects.

The second common issue is "we have a query that takes too long to perform". Whether "too long" is 5 seconds or 0.05 seconds is up to you. However, resolving this is unlikely to be as straight forward as simply caching the results. There is a myriad of approaches to take in this case. Perhaps you can async the db call and continue processing parts of the unit of work that are not dependent on the data until the data is available. Perhaps you can look at a query profiler to diagnose issues, generate indexes, or change the query to be better performing. Perhaps you need to using a data warehousing mechanism to flatten your operational/state data and provide fast immutable recorders.

Back to the code itself, your cache miss scares me slightly in that enumerating a cache with ids not correctly loaded beforehand will behave correctly (by providing the entity after roundtripping the database for it) but will do so for each yield of the enumeration. It would not be difficult for this issue to go unnoticed.

I've used generic repositories and generic caches in the past, but only for the tiniest of applications. I highly recommend creating each repository by hand. Provide usefully named methods on these bespoke repositories that wouldn't be possible with a generic approach like userRepository.GetFailedLoginsFor(userId). It makes your code more readable, thus more maintainable, and allows for case by case optimisations, only if it is required.

Tl;dr:

Sometimes it may help your team consider the new practice and therefore more carefully consider the database calls being made. But it's just syntax candy. Your practices more likely need revision.

share|improve this answer
    
Thanks Smudge. We generally are using this due to network latency, not query execution time. With regards to the cache miss - are you referring to calling GetItem() in a loop with an un-initialized cache? If so, all constructors initialize with the exception of the protected one –  Simon Aug 21 at 0:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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