Thomas Eldblom and I are reviewing most of the projects, we do in Pentia. The most of the things we comment on concerns C#, architecture, HTML, XSL or other general technologies, but some issues are related to the use of Sitecore. In this post I want to share the most common, most severe or most irritating issues/pitfalls I often stumble upon or look for, when doing a code review. I am glad to say, that I almost never stumble upon the most basic things, such as layouts specified directly on items, masters set on masters (SC5) etc.
· Use of Sitecore.Context in logic
When having actual classes or general logic, I sometimes find a reference to the Sitecore.Context. Often it is a reference to Sitecore.Context.Item or Sitecore.Context.Database. This is rarely a good idea. The logic should be independent of the context it runs in. Instead you should pass in the objects you need as parameters. If you don’t do this, you’ll bind your logic to a certain context, making it unusable, if you want to use it without a context. Further logic that uses the context isn’t very testable.
· Iterating over to many items
I don’t find this sort of issue very often, but it is pretty severe. The scenario is often that you have a list of all latest documents or a similar relational operation. You implement a simple XSL using “//item”. This works fine in development, but when the client starts importing or creating several thousand documents, the list doesn’t perform and brings the whole site down.
· Too much logic in an XSL
Often you start out with a page type, which looks like a simple rendering. Therefore you start out creating an XSL. As you dig into the specs, you find out, that you need a lot of logic. You end up with a large amount of XSL extensions and a complex rendering. The result is a way to complex XSL, which is practically unreadable and impossible to maintain. When you have a lot of logic use a sublayout.
· Reference to specific databases
Sometimes I find a reference to a specific database. For instance something like Sitecore.Configuration.Factory.GetDatabase(‘web’). Allthough it may be needed in some situations, it often gives some unintended sideeffects. Most of the time you want to operate on the context database, so you use the correct database specified in the site section in the web.config. A direct reference to the web database might cause preview functionality not to work as intended, and a direct reference to the master database might not work if you are using a staged environment.
· Null reference
As Sitecore is so type weak and everything is an item, you don’t know what fields/properties the item has. No matter how experienced the developer is, I often find the use of a field of some sort, where it hasn’t been validated whether the field is null or not, resulting in a null reference exception if the input is incorrect.
· Overriding standard Sitecore functionality
Sometimes a developer isn’t satisfied with the way Sitecore implements something – for instance the indexing, the site resolver, the domain etc. As Sitecore allows you to override almost everything, the developer thinks… Hey, I’ll just override the domain doing my own implementation. Although this might be necessary, it should be considered carefully. I have too many times stumbled into a problem with a custom implementation of standard functionality, either because it may be more complex than first assumed, the next developer on the project isn’t aware of it or last but not least an upgrade of Sitecore changes the requirements to the functionality.
I am hoping you want to share some of your experiences as well – telling what Sitecore pitfalls you often stumble upon or find very irritating.
