Solutionizing .NET

A while back I posted a rather clumsy technique to mitigate an SPWeb leak discussed here. I knew there had to be a better way, and Rob Garrett‘s use of delegates seems to have potential.

But first, I should point out a rather subtle leak in Chris’s and my code:

        list = currentContext.Site.AllWebs["MyWeb"].Lists["MyList"];

See it? How about now:

        SPWeb web = currentContext.Site.AllWebs["MyWeb"];
        list = web.Lists["MyList"];

The context SPSite shouldn’t be disposed, but AllWebs returns an SPWeb that should (according to Roger Lamb):

        using (SPWeb web = currentContext.Site.AllWebs["MyWeb"]) {
          list = web.Lists["MyList"];
        }

That Chris and I (and our readers) can overlook this in posts about SPWeb leaks is a testament to how tricky this stuff can be.

Super Delegates?

So how would Rob’s technique be used to DoSomething? Well none of his helpers take advantage of SPContext, so first let’s add a helper for that:

public static void GetContextWebByTitle(string title, Action<SPSite, SPWeb> action) {
  if (String.IsNullOrEmpty(title))
    throw new ArgumentNullException("title");
  if (null == action)
    throw new ArgumentNullException("action");

  SPContext currentContext = SPContext.Current;
  if (null == currentContext)
    throw new SPException("Context is null!");

  SPSite site = currentContext.Site;
  using (SPWeb web = site.AllWebs[title]) {
    if (null == web)
      throw new SPException("Web not found");
    action(site, web);
  }
}

Now we can refactor into a method that matches the delegate:

public void DoSomething(SPSite site, SPWeb web) {
  SPList list = web.Lists["MyList"];

  // do something with list..
  foreach (SPListItem item in list.Items) {
    processItem(item);
  }
}

And our original DoSomething() just determines which helper to call:

public void DoSomething() {
  if (SPContext.Current != null)
    SPHelper.GetContextWebByTitle("MyWeb", DoSomething);
  else
    SPHelper.GetWebByTitle("http://litwaredemo", "MyWeb", DoSomething);
}

Pretty simple. But we’re still thinking a bit too much – couldn’t GetWebByTitle check context for us? Of course:

  if (SPContext.Current != null)
    GetContextWebByTitle(title, action);
  else
    using (var site = new SPSite(url)) {
      ...

So we don’t have to think at all:

public void DoSomething() {
  SPHelper.GetWebByTitle("http://litwaredemo", "MyWeb", DoSomething);
}

I still need to try this technique in some real code, but I like the theory. It certainly makes sense to separate the logic to create and dispose SPSite/SPWeb objects from the code to manipulate them, and it’s even better to standardize that logic. But even with an arsenal of slick helpers, a leaky delegate can take us back to where we started:

public void DoSomething(SPSite site) {
  foreach (SPWeb web in site.AllWebs)
    processWeb(web);
}

Not that this diminishes the value of Rob’s solution, it just reinforces the need for developers to know the disposal patterns even with help.