very simple caching - is this an o.k. technique?

Question

I am using the following technique to cache some calls to my database - this function lives in my repository.

Public Shared Function GetByStoreURL(ByVal StoreURL As String) As Model.Partner

        Dim key As String = StoreURL
        If Current.Cache(key) Is Nothing Then
            Dim objPartner = Model.DB.Select().From(Tables.Partner).Where(Partner.Columns.StoreURL).IsEqualTo(StoreURL.ToString).And(Partner.Columns.IsDeleted).IsNotEqualTo(1).ExecuteSingle(Of Partner)()
            Current.Cache.Add(key, objPartner, Nothing, Web.Caching.Cache.NoAbsoluteExpiration, TimeSpan.FromMinutes(60), CacheItemPriority.NotRemovable, Nothing)
        End If

        Return DirectCast(Current.Cache(key), Model.Partner)

    End Function

Is this technique flawed? seems so simple and looks like it has been working great.

Answer 1

This technique is know as lazy initialization. Yes, it is widely used.

Answer 2

The only problem with this is if you want to invalidate the cache (due to changing data) and you're running on a load balanced/clustered environment. If that's a posibility then you can add a file dependency, or look at a distributed caching system like Velocity.

If you don't need to do that, you don't to scale out, and you don't mind having potentially "stale" records then that's fine :-)

Answer 3

Actually, lots of stuff wrong here with the most obvious being violation of a Single responsibility principle. Basically, your function should do only one strictly defined task (that is, retrieve a Partner from the DB).

Next goes multithreading. Web sites are inherently multithreaded, so this should be taken into account. More specifically, an item can be removed from cache between calls to If Current.Cache(key) Is Nothing and Return DirectCast(Current.Cache(key), Model.Partner). Locking is required to solve this issue, but this will bloat the function even more (see first paragraph).

Furthermore, item can be added to cache while your DB code is executed. This will result in an error, since Cache.Add does not overwrite values with the same key, which will already be there (think multithreading).

And finally I question uniqueness of a StoreURL.