eBay SharePoint 2010 – Coding Practices

When we started analysis for upgrading from SharePoint 2007 to 2010 we went through the pre-existing code base of roughly twenty solutions and dozens upon dozens of features and web parts. Much of this existing code base had known issues ranging from security (never use RunAsElevated for user facing components!), to memory leaks, to just sloppy code. Additionally we were also tasked with making all of the existing code base more performant and closer to best practices.

At a high level our goals were straight forward enough

Remove any and all security issues

Address performance of existing components

Build everything as manageable solutions (No file system edits)

Address memory leaks

Refactor as much as possible to clean up code cruft

No hard coded connections

Global CSS, JS and classes as much as possible

We started by running through a deep code review and a ranking of severity of existing issues. Chris, and I all ran through all of the existing code to get as much of the issues identified and ranked as possible. We then did two agile sprints to get all of the high severity issues fixed. From there we slowly integrated in other changes as we built up enhancements and fixes directly related to the upgrade. After fixing the basic security and performance issues we managed to achieve a 400% speed increase of some of the most commonly used custom web parts. The most common issues we encountered in the existing code base were

Misuse of SPSecurity.RunAsElevated

Fix permissions, do not immediately run to RunAsElevated to just get it working

Creating new SPSite and SPWeb objects when not needed

Use SPContext! 🙂 And don’t recreate spweb and spsites you don’t need

Not using SPContext.Current

Unless you’re running as elevated just use SPContext.Current

SPWeb.EnsureUser(); <–slowest thing ever -_-

For a custom user comment list there was three ensureusers for every action’s of every comment that caused a load time to scale from 10 seconds up to minutes

Non-optimized CAML queries

RecursiveAll or <Query></Query> when you only need a few items is not the best option even if it’s easy

SPList.Items and iterating through items and fields by hand

Was done by a .NET developer unfamiliar with SharePoint coding practices or methods

Inline styling

Easy one to fall in to but please always use classes and have your CSS outside your logic

Inline JavaScript

Also easy to fall in to, especially when you’re trying to grab tag ID’s of individual components, but remember jQuery is your friend 🙂