HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Simple Cache Mechanism

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
cachesimplemechanism

Problem

I have this simple cache mechanism for a repeated task in service. It uses static variable to store the information. Please suggest on how is it and if it could be better.

Premise: I need to verify transaction from stores. There can be lots of transactions happening repeatedly from multiple stores. Thus, this simple cache manager for getting the store information. I want it to be simple yet effective and fast.

StoreCacheManager.cs

public class StoreCacheManager
{
    private static List _merchantStores = new List();
    private DBEntities _db;
    private TimeSpan _cacheTime = new TimeSpan(1, 0, 0);//1 Hour
    public TimeSpan CacheTimeSpan { get { return _cacheTime; } }
    public StoreCacheManager(DBEntities db)
    {
        _db = db;
    }

    public async Task Get(int storeId)
    {
        if (_merchantStores.Any())
        {
            var store =
                _merchantStores.FirstOrDefault(i => i.StoreID == storeId);
            if (store != null)
            {
                // Check if Cache time has expired
                if (store.CacheDateTimeUtc.Add(_cacheTime)  GetAndCache(int storeId)
    {
        var store = await GetStoreInfo(storeId);
        if (store != null)
        {
            lock (_merchantStores)
            {
                _merchantStores.Add(store);
            }
        }
        return store;
    }
    private async Task GetStoreInfo(int storeId)
    {
        var storeInfo = await _db.Stores.Where(i => i.StoreID == storeId).Select(i => new StoreCacheInformation()
        {
            CountryCode = i.CountryObj.CountryCode,
            MerchantID = i.VendorOrgID ?? 0,
            TelephoneCode = i.CountryObj.TelephoneCountryCode,
            StoreID = storeId,
            //todo: deviceId and Token

        }).FirstOrDefaultAsync();
        if (storeInfo != null)
        {
            storeInfo.CacheDateTimeUtc = DateTime.UtcNow;
        }
        return storeInfo;
    }
}


StoreCacheInformation.cs

```
public cl

Solution

Concurrency

If you want to implement it thread-safe, you have also lock the whole transaction. For instance: store != null assumes that there is no item in cache. Imagine that after that check another thread added one. That would result in a cache where the same item is cached twice.

Conside to use a thread-safe collection (e.g. ConcurrentDictionary) instead of using locking.

.Net Framework already provides a thread-safe cache: MemoryCache.

I am not very familiar with the entity framework, but as far as I know is the DbContext not thread-safe. Therefore it is not a good idea to use a single instance of it in multithreaded environments.

Code Style

  • _merchantStores, _db and _cacheTime should be read-only.



  • Methods that return a Task should be called xxxAsync



  • The property setter in StoreCacheInformation should be private or at least internal. Otherview external code may modify the state of the cached items.



  • For many cached items, it is better to use a dictionary instead of list.

Context

StackExchange Code Review Q#134518, answer score: 6

Revisions (0)

No revisions yet.