Using ConcurrentDictionary as a cache and handling update

Question

I have the below code and now I want to add an UpdateSetting method.

The best way of doing this that I can see is via TryUpdate on the ConcurrentDictionary but that means knowing the previous value so that would require a call to GetSetting which seems a bit yucky. What are your thoughts? Is there a better way?

NOTE: If the value is not in the cache it should do nothing. On successful updating of cache it should call the settingRepository.Update

Thanks

public class MySettings : IMySettings
    {
        private readonly ISettingRepository settingRepository;
        private readonly ConcurrentDictionary<string, object> cachedValues = new ConcurrentDictionary<string, object>();


        public MySettings(ISettingRepository settingRepository)
        {
            this.settingRepository = settingRepository;
        }

        public string GetSetting(string key)
        {
            return this.GetSetting<string>(key);
        }

        public T GetSetting<T>(string key)
        {
            object value;
            if (!this.cachedValues.TryGetValue(key, out value))
            {
                value = this.GetValueFromRepository(key, typeof(T));
                this.cachedValues.TryAdd(key, value);
            }

            return (T)value;
        }

        private object GetValueFromRepository(string key, Type type)
        {
            var stringValue = this.settingRepository.GetSetting(key);
            if (stringValue == null)
            {
                throw new MissingSettingException(string.Format("A setting with the key '{0}' does not exist.", key));
            }

            if (type == typeof(string))
            {
                return stringValue;
            }

            return ConvertValue(stringValue, type);
        }

        private static object ConvertValue(string stringValue, Type type)
        {
            return TypeDescriptor.GetConverter(type).ConvertFromString(stringValue);
        }

    }

What you want your UpdateSetting method to do if the key does not exist? — hatchet - done with SOverflow, Jan 23 '15 at 18:29
Yeah I thought about that and I think the answer is nothing.For example if you update a value in a table where the key is not found via SQL it would do nothing so I thought same applies here — Jon, Jan 23 '15 at 18:31
You could just do `cachedValues[key]=newValue` and catch the exception if the key doesn't exist. — hatchet - done with SOverflow, Jan 23 '15 at 18:32
What if you have 2 threads trying to do the same? I guess the latter would win so may not be a big issue? — Jon, Jan 23 '15 at 18:34
One will win and one will lose, but that's going to be the case however you do it, I think. — hatchet - done with SOverflow, Jan 23 '15 at 18:35
But I think there is more to it. If it is not in the cache then you retrieve the value from the repository. At that point you can compare the value and from the repository update the repository or not. If not in the repository then done. If it is in the cache and equal then done. In the case in the cache and not equal then update both. — paparazzo, Jan 23 '15 at 18:53
You know, you could just use a regular dictionary and a lock object. You'd be guaranteed to have the expected behavior. What you're doing here (read/write to a dictionary) really doesn't take many cycles at all. Hell, you might save a few going with a simple lock. — , Jan 23 '15 at 18:56

hatchet - done with SOverflow · Answer 1 · 2015-01-23T19:09:46.563

One way would be to simply set the value, and catch the exception that it will throw if the key is not in the collection.

    public bool UpdateSetting<T>(string key, T value)
    {
        try {
            this.cachedValues[key] = value;
        } catch (KeyNotFoundException ex) {
            return false;
        }
        return true;
    }

Whether this is how you want to handle the key not existing is up to you. But if you decide you want to add the key, then you should use the AddOrUpdate method rather than the simple assignment above. And you won't need to catch that exception in that case.

To address the bigger issue of your backing repository, I think you'll need something along the lines of this.

    public bool UpdateSetting<T>(string key, T value)
    {
        lock {
            try {
                this.cachedValues[key] = value;
                this.settingRepository.Update(... //you'll have to write this
            } catch (KeyNotFoundException ex) { 
                return false;
            }
            return true;
        }
    }

I don't think you can avoid the use of a lock to ensure the change to the cache matches the repository. For the same reason, I think some of your existing code may need locking as well. Using ConcurrentDictionary only protects you in the operations on the dictionary. But in the larger scope, there is more going on that needs synchronization.

I was just addressing the question of TryUpdate. To do the full implementation in your class, I think you'll have to wrap it all in a lock and update the repository if successful in updating the cache. Which raises the question of what to do if it's not in the cache. I assume you want to update the repository regardless. You should add those details to your question. — hatchet - done with SOverflow, Jan 23 '15 at 19:00
Thanks. I don't think you need a lock on getting the value as its not changing the value in the dictionary? — Jon, Jan 24 '15 at 09:27

paparazzo · Accepted Answer · 2015-01-23T19:22:29.697

Might be worth getting the existing value to possible avoid updating the repository. And an exception if more expensive than a try

public bool UpdateSetting<T>(string key, T value)
{
    lock 
    {
        T oldValue;            
        if (this.cachedValues.TryGetValue(key, out oldValue)
        {
            if (oldValue != value)
            {
                this.cachedValues[key] = value;
                settingRepository.Update(key, value);
            }
            return true;
        } 
        else 
        { 
           return false;
        }            
    }
}

Using ConcurrentDictionary as a cache and handling update

2 Answers2