Instantiating SPListItems in Event Receivers - Best Practices

Question

In the MSDN article Best Practices with Event Receivers it is clearly stated:

Do not instantiate an SPWeb, SPSite, SPList, or SPListItem object within an event receiver.

The article goes on to describe how to get references to the item being updated via the event properties.

Say I need to check a value of an SPListItem from a different SPList and a different SPWeb (but within the same SPSite) to conditionally format the item triggering the event receiver.

Is this a grave sin to do this like:

using (SPSite site = new SPSite(properties.SiteId))
{
    using (SPWeb web = site.OpenWeb("WebName"))
    {
         SPList list = web.GetList("/Path");
         SPListItem item = list.GetItemById(theID);
    }
}

Edited for Clarity

My questions are:

1) What issues would this cause aside from performance?

2) Does this only cause issues if you instantiate the item that triggered the event receiver or does this cause an issue if you instantiate any of these objects?

Answer 1

About your code: Since you are checking the value of an SPListItem in a different SPWeb but in the same SPSite, I think something like this will be better:

using (SPWeb web = properties.Site.OpenWeb("WebName"))
{
     SPList list = web.GetList("/Path");
     SPListItem item = list.GetItemById(theID);
}

From the MSDN article you posted:

Do not instantiate an SPWeb, SPSite, SPList, or SPListItem object within an event receiver. Event receivers that instantiate these objects instead of using the instances passed via the event properties can cause the following issues:

Significant additional roundtrips to the database (one write operation can result in up to five additional roundtrips in each event receiver).

So performance is the key issue over here and the most important one according to me. Also, if you are creating a new instance of the SPListItem, SPWeb or SPSite, you have 2 instances being updated simultaneously withing the EventReceiver. (The instance which you already have through the properties and the new instance which you created. That is why it is important to call the Invalidate methods on the newly created instances)

In your case, you do not already have the SPWeb and SPListItem instance of the item which you want to get the value of. So naturally you will want to create new instances of these to get the desired value. The "Do not create new instances in Event Receivers" rule only applies if you already have the desired instances being passed through the properties object.

If you already have the instances you want, why do additional database calls to get them again? But if you do not have what you want, doing a database call is the only option to get it right?

Answer 2

that coding practice described there is valid for the 'ing' suffixed events. That event 'ItemDeleting' occurs when the item is being deleted as opposed to 'ItemDeleted' which happens after the item was deleted. If you instantiate a SPSite inside an 'ItemDeleted' event, you're safe because that event isn't calling Update. It's best if you can to avoid using those events and stick to '..deleted' or'..added' and avoid excess logic in the event receiver and keep as much as you can in the business layer of the app. Also, performance problems related to the events are pretty painful, especially those related to synchronous events (for example checking something against a record in the DB within a synchronous event). Hope that helped.