Code Reviews are interesting – both sides can learn something, and the one I’ve just done taught me a few things…

The difference between Convert.ToInt32() and int.Parse()

Both are ways of taking a string, and trying to turn it into an integer. Both throw exceptions for invalid formats. However, Convert.ToInt32(null) returns zero, while int.Parse(null) throws an exception still. (Personally, I prefer the int.Parse()). The same seems to be true for other types.

An added this to note is that the primitive type conversions normally have a .TryParse() method too.

Use SPQueries … to create new ListItems

Yes, it sounds weird, but it’s true – it’s more efficient to run an empty query to get an SPListItemCollection to add the new SPListItem to, than to use SPList.Items.Add(). So…

const string EmptyQuery = "0";
SPQuery q = new SPQuery {Query = EmptyQuery};
return list.GetItems(q).Add();

The reason is that using SPList.Items.x will (internally) get all items in the list. That can be slow. It’s a shame there isn’t an SPList.AddItem() function to encapsulate this. I already knew this tip, but had forgotten.

Use SPItemEventProperties.OpenWeb()

I’d forgotten about this one entirely – the event properties parameter for an event handler has a method for getting the SPWeb. I’d a habit of using the IDs of the SPWeb and SPSite object (which are also in the properties) to create my own instances of the objects.

Actually, this isn’t always a bad thing – often I find my event handlers have to use elevated code, and you don’t want to pass SPWeb or SPSite objects into an elevated code block – as their security context remains unchanged, and you can get some weird results. In that case, it is better to get the IDs and pass them in.