Friday, March 25, 2011

FxCop: Properties should not return arrays

Did you run FxCop and it reported a “Performance” issue regarding “Properties should not return arrays”?
Well, don’t be in a hurry to fix it just yet.
After an online meeting with Microsoft online tech team, I was surprised to hear them claim that this may pose a performance issue, since developers “must return a copy of the array”, which will cause a performance issue when accessing this property excessively (like via an indexer, in a loop).
They went on later to recommend changing this property into a method, to make it clear for the developer who is using it that he is better off calling the method once and storing it in a local member, rather then using the indexer on the property itself, Or change the property type into a collection. There is even an example at the end of the page of converting the array into a collection, which “resolves” this “performance” issue.
Also, from reading the rule page on MSDN, it seems this issue only refers to a possibly problem of breaking the read-only attribute of an array that is returned as a property, which would unintentionally allow modifications to it. Definately not what I was told in that meeting, and besides - when the array is not read only I don't see the problem here. Isn't it up to the architect to decide what to use, and if the array should or should not be read-only?
Let me give you a few points to think about before complying to this rule:
1. Properties do not serve the same purpose as methods. If you need to serialize this class, a property would be serialized while a method would not – resulting in loss of data upon serialization. This is extremely important when talking about web part properties. Even changing the property into a read only collection will not help, as XmlSerializer will throw an exception on that property.
2. An array does not serve the same functionality as a collection. Especially when reflecting it, or serializing it for use later (JSON serialize / SOAP serialize in web service for example).
3. If you have an existing web part that uses an array as property, DONT CHANGE IT! you cannot change it’s type, nor change it into a method! Doing so will break all your existing web parts in all pages and you will have to remove them and re-configure them, page by page. This goes to web parts, but also to any serialized content you have of existing code.
4. Speaking of performance issue, the author assumes that *if* you build the array on every get_Property access this will have negative performance. But – why would you do that? In most you just return a private array member that should not be read-only, which does not affect performance at all.
5. Even more, by implementing the resolution suggested and converting into a collection, performance will have a far worst negative implication! Working with arrays is with no doubt the fastest way of working with a collection in .NET! It is faster than any other collection type, including Generic collections, in every possible way.
Consider a simple “for” loop, using “items[i]” <—an array will beat any collection any time, any place!
So, by suggesting to fix a non-existent “performance” issue, you actually cause a much greater real “performance” issue on your application.
What I don’t understand is, this rule has been there since 2006 with many people posting comments on its validity, Why is it still out there?
The worst thing about it, with Microsoft SharePoint online hosting, they require your to fix all of these warnings on your code. How can this be a requirement if this is clearly a design consideration and not a clear-cut case of issue to be fixed?
I would suggest you take any tool (FxCop or any other automated tool) as suggestion only, and use your own discretion on when or if to apply these rules.

Tuesday, March 1, 2011

Parser Error – SharePoint crash after deployment

Today I run into a strange problem.

I build a new version of a product and installed it – and it crashed my entire SharePoint:

image Now, I want to say that this does not happen often, but I fear it does… :)

The error reads:

Parser Error

Description: An error occurred during the parsing of a resource required to service this request. Please review the following specific parse error details and modify your source file appropriately.
Parser Error Message: This page has encountered a critical error. Contact your system administrator if this problem persists.

Source Error:

Line 1:  <%@ Page Inherits="Microsoft.SharePoint.Publishing.TemplateRedirectionPage,Microsoft.SharePoint.Publishing,Version=,Culture=neutral,PublicKeyToken=71e9bce111e9429c" %> <%@ Reference VirtualPath="~TemplatePageUrl" %> <%@ Reference VirtualPath="~masterurl/custom.master" %>
Line 2: <html xmlns:mso="urn:schemas-microsoft-com:office:office" xmlns:msdt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882"><head>
Line 3: <!--[if gte mso 9]><xml>

Source File: /Pages/Default.aspx    Line: 1

Well, I started investigating this strange error message…

The weird thing was, all my SharePoint sites reported this error except for central administration – which worked just fine!

After a short research I noticed in SharePoint Log this strange message, that my web part DLL was throwing an exception that it could not load a referenced file: Microsoft.SharePoint.ApplicationPages.Administration

Well… this makes sense, since this file only exists in central administration app_bin folder.

Things started to come together. All I had done wrong was to use this dll in my code, since I had some code that was running in central admin (settings page)

Well, as soon as I removed it from my references everything started working again.

Now I have to find another way to create a settings page, since GlobalAdminPageBase base class (for central admin settings pages) is something I cannot use.

Either that or I will have to separate my settings page into another DLL (which I prefer not to do)

Well, if you get this error message – be sure to check SharePoint logs and make sure you are not using this dll!