16 Jul 2010

If you worked a bit with SharePoint, you surely know how important it is to dispose of SPSite objects, SPWeb objects and such.

However, these objects are sometimes retrieved by using another object’s property. A very common one is SPContext.Current that has properties to return the current SPSite and SPWeb.

Needless to say, calling dispose on an object that you got via another object’s property seems utterly rude. I mean, you are basically disposing something that was “borrowed” from another object. When I see code doing this, I get very suspicious.

Here is a common example:

using (SPWeb web = SPContext.Current.Web)
{
    //Do something with web
}

Using reflector, here is what SPContext.Web does:

public SPWeb Web
{
    get
    {
        if (this.m_web == null)
        {
            this.m_web = SPControl.GetContextWeb(this.m_context);
        }
        return this.m_web;
    }
}

As we can see, this returns a private field of type SPWeb. Calling Dispose() on that (via the using statement) is clearly not very nice to our beloved SPContext. This kind of coding is known to bring unexpected errors. In fact, FxCop will raise a warning if it is set to check for SharePoint Best Practices Rules.

If this is a general rule, there are of course exceptions, I believe due to some inconsistencies in the API. Here is one of them:

SPWeb web = SPContext.Current.Web;

foreach (SPWeb subWeb in web.Webs)
{
    //Do something with subWeb

    subWeb.Dispose();
}

In this particular case, we iterate over a collection of SPWeb objects trough the SPWeb.Webs property. As said before, at first sight, this looks bad.

Again, using reflector, this is what the Webs property does:

public SPWebCollection Webs
{
    get
    {
        if (this.m_Webs == null)
        {
            this.m_Webs = new SPWebCollection(new SPWebCollectionProvider(this), this.m_guidId);
        }
        return this.m_Webs;
    }
}

It is very similar to the previous code, so it feels very unnatural to call Dispose() on all object that are held in that collection.

However, this is the way to go, otherwise FxCop will raise a warning saying that SPWeb objects were not disposed of. In my opinion, this shouldn’t be a property, but a method called GetWebs() or something similar, as explained in Framework Design Guidelines.

Related links:



blog comments powered by Disqus