Is this modified C # syntax pattern good practice?

Question

I am developing a blog application that provides non-profit organizations. I want each organization to be able to change their own blog settings. I took the singleton template (from BlogEngine.net) and changed it. (I understand that this is no longer a singleton pattern.) I tested this approach and seems to work fine in a development environment. Is this model a good practice? Are there any problems that may arise when they are placed in a production environment?

public class UserBlogSettings { private UserBlogSettings() { Load(); } public static UserBlogSettings Instance { get { string cacheKey = "UserBlogSettings-" + HttpContext.Current.Session["userOrgName"].ToString(); object cacheItem = HttpRuntime.Cache[cacheKey] as UserBlogSettings; if (cacheItem == null) { cacheItem = new UserBlogSettings(); HttpRuntime.Cache.Insert(cacheKey, cacheItem, null, DateTime.Now.AddMinutes(1), Cache.NoSlidingExpiration); } return (UserBlogSettings) cacheItem; } } } 

(Code particles were omitted for brevity.)

Thanks for any help, comment, etc.

Answer 1

If this is a session, save it in the session, not in the cache.

In addition, you do not expect an increase or decrease in the rating:

 object cacheItem = HttpRuntime.Cache[cacheKey] as UserBlogSettings; 

removes unnecessary roles

 UserBlogSettings cacheItem = HttpRuntime.Cache[cacheKey] as UserBlogSettings; if (cacheItem == null) { cacheItem = new UserBlogSettings(); HttpRuntime.Cache.Insert(cacheKey, cacheItem, null, DateTime.Now.AddMinutes(1), Cache.NoSlidingExpiration); } return cacheItem; 

Answer 2

You need to use a lock to avoid possible race conditions:

  private static Object lock_Instance = new Object (); public static UserBlogSettings Instance { get { string cacheKey = "UserBlogSettings-" + HttpContext.Current.Session["userOrgName"].ToString(); UserBlogSettings cacheItem = HttpRuntime.Cache[cacheKey] as UserBlogSettings; if (cacheItem == null) { lock (lock_Instance) { // need to check again in case another thread got in here too cacheItem = HttpRuntime.Cache[cacheKey] as UserBlogSettings; if (cacheItem == null) { cacheItem = new UserBlogSettings(); HttpRuntime.Cache.Insert(cacheKey, cacheItem, null, DateTime.Now.AddMinutes(1), Cache.NoSlidingExpiration); } } } return cacheItem; } } 

Answer 3

I think you are good overall, but I would suggest a performance improvement if it becomes necessary (I know ... don't optimize until you really need it).

I would probably implement this with such a method to get the settings object:

 public static UserBlogSettings getSettings(string orgName, Cache cache) { // do the same stuff here, except using the method parameters } 

The reason for this is that HttpContext.Current and HttpRuntime.Cache must go through some vibrations to get the current session and cache handles. If you call this from the asp.net page, you already have something in your hand. Therefore, use the ones you already have, and do not view them again.