Summary: Common Coding Issues

2009-04-08

The Best Practice MSDN page has some interesting hints to generate better code. In this post I write about some point of that article, provide samples and fix bugs which are in the MSDN article.

Caching

You should only cache thread safe objects. What’s that?

This means, that you should only cache objects, which can not be changed from the outside of your code. An itemCollection (as list.Items) is changed, if another user adds an item. But if you cache a DataTable which you can get from an itemCollection with list.Items.GetDataTable() it will not be changed later on, and can be cached.

A lock should be put around reading the items and caching them. This will ensure that for the reading process, the items are not changed from the outside.

 1: private static object _Lock = new object();

<span class=lnum>   2:  </span>

 3: public void CacheData(SPList list)

<span class=lnum>   4:  </span>{

 5:  SPListItemCollection listItems;

<span class=lnum>   6:  </span>    <span class=kwrd>lock</span> (_Lock)

 7:  {

<span class=lnum>   8:  </span>        <span class=rem>// Cache is your caching mechanism</span>

 9:  DataTable dataTable = (DataTable) Cache["ListItemCacheName"];

<span class=lnum>  10:  </span>        <span class=kwrd>if</span> (dataTable == <span class=kwrd>null</span>)

 11:  {

<span class=lnum>  12:  </span>            listItems = list.GetItems(<span class=kwrd>new</span> SPQuery {RowLimit = 0});

 13:  dataTable = listItems.GetDataTable();

<span class=lnum>  14:  </span>            Cache.Add(<span class=str>"ListItemCacheName"</span>, dataTable)

 15:  }

<span class=lnum>  16:  </span>    }

 17: }

Event Receivers

Use the objects provided by the SPItemEventProperties parameter of your event method.

SPWeb web = properties.OpenWeb();

instead of

using (SPSite site = new SPSite(properties.WebUrl))
using (SPWeb web = site.OpenWeb())

Do not use SPList.Items

The Items property will return every list item. And it will read all items for every call to it. So every time your code has an .Items, a SQL query is generated, which reads all items. They have to be fetched, and stored in memory.

SPList.Items.Add()

Instead use SPList.AddItem we can not use SPList.AddItem, since it does not exist 🙁
So we query the list for 0 rows, and add a new list item to the itemCollection with 0 elements.

 1: SPQuery query = new SPQuery {RowLimit = 0};

<span class=lnum>   2:  </span>SPListItem newItem = list.GetItems(query).Add();

 3: newItem[SPBuiltInFieldId.Title] = DateTime.Now.ToString();

<span class=lnum>   4:  </span>newItem.Update();

Compared with list.Items.Add() the code runs significant faster. In my case I tested against a list with about 1100 items. A trace with the SQL Server Profiler showed, that the code took 19983 microseconds. Adding an item to SPList.Items took 288996 microseconds. This is 14 times more!

Get Items by Identifier

If you have an ID, get your list item through list.GetItemsByID(id). Otherwise use a SPQuery instead of iterating through all list items (see above).

Use paged Queries

The MSDN article provides some samples on how to get all list items. Not all at once, but 2000 at a time.

Use list.ItemCount instead of list.Items.Count
Use the PortalSiteMapProvider class (MOSS only!)

The PortalSiteMapProvider class uses caching by default. So if you need to query the same list, and it does not change frequently, you can increase the performance easily.

You can use the PortalSiteMapProvider for WSS sites, if you have MOSS installed. There is no need to activate MOSS features on the site, where you want to use the caching.

Also think about security trimming when you query for items, as you might only get a subset of all items in the initial load, if the user may not access all items!

 1: SPWeb web = SPContext.Current.Web;

<span class=lnum>   2:  </span>SPList list = web.Lists.Cast<SPList>().SingleOrDefault(

 3:  l => l.Title == "ListName");

<span class=lnum>   4:  </span><span class=kwrd>if</span> (list == <span class=kwrd>null</span>) <span class=kwrd>return</span>;

5:

<span class=lnum>   6:  </span>PortalSiteMapProvider provider = PortalSiteMapProvider.WebSiteMapProvider;

 7: PortalSiteMapNode webNode =

<span class=lnum>   8:  </span>    (PortalSiteMapNode)provider.FindSiteMapNode(web.ServerRelativeUrl);

 9: if (webNode == null || webNode.Type != NodeTypes.Area) return;

<span class=lnum>  10:  </span>

 11: Stopwatch watch = new Stopwatch();

<span class=lnum>  12:  </span>watch.Start();

 13: SPQuery query = new SPQuery { Query = list.DefaultView.Query };

<span class=lnum>  14:  </span>SiteMapNodeCollection items = provider.GetCachedListItemsByQuery(

 15:  (PortalWebSiteMapNode)webNode, list.Title, query, web);

<span class=lnum>  16:  </span>

 17: foreach (PortalListItemSiteMapNode item in items)

<span class=lnum>  18:  </span>{

 19:  string title = (string)item[SPBuiltInFieldId.Title];

<span class=lnum>  20:  </span>}

 21: watch.Stop();

<span class=lnum>  22:  </span>Controls.Add(<span class=kwrd>new</span> LiteralControl(

 23:  "Items found: " + items.Count + " in " + watch.ElapsedMilliseconds + "ms"));

In the example, the stopwatch got about 2000 ms for the initial load. For each access to the items after they have been cached, it took 15-20 ms. So it is roughly 100 times faster!
You might have to adjust the query to your needs, since it uses the default view to get the items.

Conclusion

Review your code for memory leaks (SharePoint Diagnostics Tool (SPDiag)) and follow common coding issues to generate clean, safe and fast SharePoint code.