4
\$\begingroup\$

I haven't done much .NET development in several years. Now I'm enjoying some of the new stuff with modern .NET. I just want to make sure I'm using things correctly. I'm a solo developer with no one else to bounce ideas off of.

I have a new MVC web application. I'm using the Massive micro-ORM. I'm a big believer in caching database requests. I've also created an EnumWrapper class.

Does anything look terrible in this code?

My caching class:

using System;
using System.Runtime.Caching;

namespace ULH.Common
{
    public static class Caching
    {
        private static readonly object _cacheLock = new object();

        public static dynamic LoadFromCache(string cacheKey, int secondsToCache, Func<object> query)
        {
            object result;

            if (secondsToCache == 0)
            {
                // if we're not caching, clear the cache and return result directly
                MemoryCache.Default.Remove(cacheKey);
                result = query.Invoke();
            }
            else
            {
                // otherwise do the caching, but use a double-check lock
                result = MemoryCache.Default[cacheKey];
                if (result == null)
                {
                    lock (_cacheLock)
                    {
                        result = MemoryCache.Default[cacheKey];
                        if (result == null)
                        {
                            result = query.Invoke();
                            MemoryCache.Default.Add(cacheKey, result, DateTime.UtcNow.AddSeconds(secondsToCache));
                        }
                    }
                }
            }

            return result;
        }

    }
}

An example of one of my data model objects:

using System;
using ULH.Common;

namespace ULH.Intranet.Chaplaincy
{
    public class EncounterTypes : BaseModel
    {
        private EncounterTypes()
        {
            this.TableName = "EncounterTypes";
            this.PrimaryKeyField = "Id";
        }

        public static dynamic GetActive()
        {
            return Caching.LoadFromCache("EncounterTypes_GetActive", 120,
                () => 
                {
                    dynamic tbl = new EncounterTypes();
                    return tbl.Find(Enabled: 1); 
                });
        }

        public static dynamic GetById(int id)
        {
            string cacheKey = "EncounterTypes_GetById_" + id.ToString();

            return Caching.LoadFromCache(cacheKey, 600,
                () =>
                {
                    dynamic tbl = new EncounterTypes();
                    return tbl.First(ID: id);
                });
        }

    }
}

Here is my EnumWrapper class, where I also cache the List that is created:

using System;
using System.Collections;
using System.Collections.Generic;

namespace ULH.Common
{

    public sealed class EnumWrapper : ReadOnlyCollectionBase
    {
        Type _enumType;
        Type _basetype;

        public static EnumWrapper Wrap(Type type)
        {
            if (type == null || !type.IsEnum)
                throw new ArgumentException("Type must be an enum", "type");

            return Caching.LoadFromCache("EnumWrapper:" + type.ToString() , 600,
                () =>
                {
                    return new EnumWrapper(type);
                });
        }

        private EnumWrapper(Type type)
        {
            _enumType = type;
            _basetype = Enum.GetUnderlyingType(type);

            foreach (var item in Enum.GetValues(_enumType))
            {
                this.InnerList.Add(
                    new KeyValuePair<object, string>(
                        Convert.ChangeType(item, _basetype), 
                        item.ToString()
                        )
                    );
            }
        }
    }
}

I'm using dynamic quite a bit due to the Massive ORM.

This won't be a huge system, so I don't want to deal with IOC containers. I'll have one common library (ULH.Common) with caching, the Massive ORM, EnumWrapper, and other common functions. There will then be several small websites using that library.

Suggestions? Does everything look okay?

Edit: George Howarth made some interesting points. I was unaware that MemoryCache was thread-safe. I've now modified my LoadFromCache() method to this:

public static dynamic LoadFromCache(string cacheKey, int secondsToCache, Func<object> query)
{
    object tocache = null;

    // result will always get the value here
    // tocache will only have the value when it was pulled from our method
    object result = MemoryCache.Default[cacheKey] ?? (tocache = query.Invoke());

    if (secondsToCache > 0)
    {
        if (tocache != null) // only save to cache if it wasn't there
            MemoryCache.Default.Add(cacheKey, tocache, DateTime.UtcNow.AddSeconds(secondsToCache));
    }
    else
    {
        // remove from cache only if secondsToCache was zero or less
        MemoryCache.Default.Remove(cacheKey);
    }

    return result;
}
\$\endgroup\$
  • \$\begingroup\$ A question. If you are loading the enum from cache what happens if two different objects with the same enum type get cached. Will you lose information as one will overwrite the other? \$\endgroup\$ – dreza May 6 '14 at 20:22
  • \$\begingroup\$ @dreza, My EnumWrapper class is used to store an instance of the enum for populating drop-downs and other UI widgets. It's selected value is not saved. \$\endgroup\$ – David Crowell May 7 '14 at 11:38
5
\$\begingroup\$
  • MemoryCache is actually thread-safe, so you don't need a double-check lock

    public static class Caching
    {
        public static dynamic LoadFromCache(string cacheKey, int secondsToCache, Func<object> query)
        {
            object result = query.Invoke();
    
            if (secondsToCache == 0)
            {
                // if we're not caching, clear the cache and return result directly
                MemoryCache.Default.Remove(cacheKey);
            }
            else
            {
                if (!MemoryCache.Default.Contains(cacheKey))
                {
                    MemoryCache.Default.Add(cacheKey, result, DateTime.UtcNow.AddSeconds(secondsToCache));
                }
            }
    
            return result;
        }
    }
    

Edit: Updated answer about the if statement, wasn't thinking straight.

\$\endgroup\$
  • \$\begingroup\$ Your changes always do the query.Invoke(), which could be an expensive operation. I was unaware that MemoryCache was threadsafe. I checked MSDN, and am happy to confirm it. I'm editing my post to show my new LoadFromCache() method. \$\endgroup\$ – David Crowell May 6 '14 at 13:14
  • \$\begingroup\$ Yeah, looking at your code again it looks like a mistake. I guess you could call Contains first before adding. \$\endgroup\$ – George Howarth May 6 '14 at 13:16
  • \$\begingroup\$ Your edited version will return null when the value is cached. Go ahead. Have another cup of coffee. :) \$\endgroup\$ – David Crowell May 6 '14 at 13:20
  • \$\begingroup\$ Yes this is what happens when you can't rely on ReSharper... \$\endgroup\$ – George Howarth May 6 '14 at 13:23
0
\$\begingroup\$

I found a huge problem with the architecture I was working on. The cached object is still an object inheriting from DynamicModel - part of the Massive ORM.

This led to the database being queried even from a cached collection. That defeats the purpose of caching.

One way of handling this would be to add a new method to create ExpandoObjects (and collections of them) that did not inherit from DynamicModel. This seems more effort than it's worth. I'm still trying to determine my best course of action.

\$\endgroup\$

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.