1

I'm building an intranet using C# webforms. I've got a list object with a bunch of users which I'm cacheing. I'm trying to create a constructor that will do the following when I reference MainADList:

  1. if it exists in the cache and is not null, use it
  2. else generate the list and cache it

I've got the code to do the caching and retrieving, but it isn't encapsulated nicely in a way that I'd like.

public Users MainADList = new Users();

private void GenerateADList()
{
    MainADList = (Users) Cache["MainADList"];

    if (MainADList == null || MainADList.Count == 0)
    {
       //generate the list....

       Cache["MainADList"] = MainADList;
    }
 }

Thanks!

Ilya Ivanov
  • 23,148
  • 4
  • 64
  • 90
user948060
  • 953
  • 3
  • 12
  • 25

3 Answers3

6

You can't create a constructor which does that. A constructor always creates a new object.

Instead, create a static factory method:

public static Users GetUsers()
{
    // Consult the cache, and create a new instance if necessary.
}

This may be a singleton - but it certainly doesn't have to be. (I wouldn't artificially impose its singleton-ness unless I really had to. I'm not a big fan of the singleton pattern.)

Alternatively, instead of a static factory method, you could have an instance method in a factory class:

public class UsersFactory
{
    // Construct it with a cache, or whatever's required

    public Users GetUsers()
    {
        // Use the cache, or construct a new value.
    }
}

Now this is more testable - because you're not relying on any static (global) state. Instead, you can create a new factory from a new cache in each test.

In all of these solutions, you need to consider what threading behaviour you want. If you want to make sure that you only construct the value once, you'll need to use Lazy<T>, static initializer guarantees, or locking.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Singleton: the most easily abused design pattern known to programming – JerKimball Jan 15 '13 at 17:47
  • Right, I've read the same thing about Singletons. What's an alternative in my case? – user948060 Jan 15 '13 at 17:48
  • Jon, so each time I need my list I'll call GetUsers()? I was hoping there'd be an inherent call to GetUsers() when I reference my object... – user948060 Jan 15 '13 at 17:48
  • Well if singleton's are bad, then what Jon Skeet proposed would be the right choice,so basically a factory method in the class. – dutzu Jan 15 '13 at 17:48
  • @user948060: Why would you want to hide the call? See my edit for an alternative to having a static factory method - have a factory class instead. – Jon Skeet Jan 15 '13 at 17:49
  • @JonSkeet I don't want to hide it so much as not have to explicitly call it each time. What do you think of dasblinkenlight's suggestion below? – user948060 Jan 15 '13 at 17:52
  • @user948060: Personally that looks messier to me than using a simple method call. (You realize you'd have that code every time you wanted to get at the value?) – Jon Skeet Jan 15 '13 at 17:55
  • @JonSkeet The rationale to using any paradigm over another is communication. You, as a team, might simply agree that MyClass.Current can be used more recklessly, without taxing the database, whereas MyClass.GetCurrent() may or may not tax the database. – svidgen Jan 15 '13 at 18:00
  • @JonSkeet There's certainly good rational in "surfacing" hidden method calls. But, I might also be inclined to argue most of your method calls and overhead are hidden anyway. – svidgen Jan 15 '13 at 18:01
  • @svidgen: I don't see how your comment really applies. I wasn't discussing the benefits of a property vs a method - I was discussing `factory.GetUsers()` vs '((Lazy) Cache["MainADList"]).Value' - the latter simply looks messier to me. – Jon Skeet Jan 15 '13 at 18:02
  • @JonSkeet My bad. I think I glossed over a comment or two. +1 – svidgen Jan 15 '13 at 18:05
  • Regarding threading, what's the recommended way? This is an intranet with users from active directory; the directory doesn't change often, and I was thinking to have the object recreated every hour or so. – user948060 Jan 15 '13 at 18:51
  • Why I am having trouble accessing Cache from my Users class? – user948060 Jan 15 '13 at 19:20
  • @user948060: Were you using `Page.Cache` before? If so, it makes sense that you can't do that from `Users`. There may be a way of getting at the cache some other way - I'm not sure. – Jon Skeet Jan 15 '13 at 19:26
  • @user948060: Yup, `HttpContext.Current.Cache` is probably what you want. – Jon Skeet Jan 15 '13 at 19:27
  • Where did dasblinkenlight's answer go?? – user948060 Jan 15 '13 at 20:30
  • @user948060: I think he deleted it. – Jon Skeet Jan 15 '13 at 20:31
  • @JonSkeet, I need a bit more help. I'm a bit confused. I like your solution, but I need to take threading into account, and I'm not sure which way to go. Assuming I want to refresh the object every 30 minutes, how/where do I do that? – user948060 Jan 15 '13 at 20:41
0

One general pattern you could follow:

public class ClassName {
  public static Object CachedObject {
    get {
      Object o = (Object)Cache['CacheKey'];
      if (o == null)
      {
         o = GetData();
         Cache["CacheKey"] = o;
      }
      return o;
    }
  }
}

And treat ClassName.CachedObject as though it's always, eternally, and magically populated.

svidgen
  • 13,744
  • 4
  • 33
  • 58
  • Note that this implementation of the singleton pattern (which is what this is, but using a cache instead of a field) isn't thread-safe. I dislike singletons due to the lack of testability. I dislike this implementation for its lack of thread safety. See the article linked in my answer for alternative implementations. – Jon Skeet Jan 15 '13 at 18:03
  • My solution is intended to mirror the OP's question, assuming the `Cache` in question is a stand-in for "some suitable caching mechanism." – svidgen Jan 15 '13 at 18:09
  • @JonSkeet Though ... it just occurred to me, there is still a locking (or lack of) problem. That said, I think I'll leave the answer as-is. Your answer is plenty sufficient! – svidgen Jan 15 '13 at 18:14
-3

What you want is known as a Singleton.

Basically what you should do with the code you already have is something like this:

public static GetList
{
   get 
      {
          //check if list exists and create it - so basically call your private constructor
          //return cached list
      }
}
dutzu
  • 3,883
  • 13
  • 19