Programming technique the CKS:EBE Team should consider

Topics: Enhanced Blog Edition
Feb 22, 2008 at 10:32 PM
Edited Feb 22, 2008 at 10:32 PM
First let me say that the CKS:EBE is a very good framework. I applaud the work you guys are doing. I like the project some much that I using it for a blog site at the company where I work and will definitely post a link to it when completed.
The project though could use some tweaking for better performance.
I believe that if the changes suggested below were made throughout the entire project, the performance for CKS:EBE would increase. I am not saying it is slow, 'cause it is not.
I have no problem making the changes suggested below if I was assured that it would get implemented in the CKS:EBE project if I upload it.

Here are the proposed changes.

1. Use StringBuilder when possible.
E.g. The DefaultUrlProvider class would benefit greatly if the StringBuilder was used to concatenate strings together. See below

StringBuilder sb = new StringBuilder();
public string createTagUrl(string tag)
{
//return ServerRelativeUrl + "/archive/tags/" + tag + "/default.aspx";
if (sb.Length > 0) sb.Remove(0, sb.Length);
sb.Append(ServerRelativeUrl);
sb.Append("/archive/tags/");
sb.Append(tag);
sb.Append("/default.aspx");
return sb.ToString();
}

public string createMonthUrl(string postDate)
{
//return ServerRelativeUrl + "/archive/" + postDate.Substring(0, 4) + "/" + postDate.Substring(5, 2) + ".aspx";
if (sb.Length > 0) sb.Remove(0, sb.Length);
sb.Append(ServerRelativeUrl);
sb.Append("/archive/");
sb.Append(postDate.Substring(0, 4));
sb.Append("/");
sb.Append(postDate.Substring(5, 2));
sb.Append(".aspx");
return sb.ToString();
}

public string createPostUrl(string postDate, string urlTitle, string postID)
{
//return ServerRelativeUrl + "/archive/" + postDate.Substring(0, 4) + "/" + postDate.Substring(5, 2) + "/" + postDate.Substring(8, 2) + "/" + urlTitle + ".aspx";
if (sb.Length > 0) sb.Remove(0, sb.Length);
sb.Append(ServerRelativeUrl);
sb.Append("/archive/");
sb.Append(postDate.Substring(0, 4));
sb.Append("/");
sb.Append(postDate.Substring(5, 2));
sb.Append("/");
sb.Append(postDate.Substring(8, 2));
sb.Append("/");
sb.Append(urlTitle);
sb.Append(".aspx");
return sb.ToString();
}

2. I notice that the string.Format method is used quite often too. That can be replaced with StringBuilder.AppendFormat

3. Use #if DEBUG ... #endif around all debugging and tracing statements. That way when the code gets compile for production (release) all debugging and tracing statements are ignore and won't affect the performance of this very good application.

4. The RewriteTagUrl method is doing unnecessary checking. See the ‘if statement’ below. This method could also used the StringBuilder
foreach (string sKey in context.Request.QueryString.AllKeys)
{
if (sKey.ToLower() != "c" && sKey.ToLower() != "c")
sQuery += sKey + "=" + context.Request.QueryStringsKey + "&";
}

5. The RelatedPosts and other class is using the Stringbuilder but in an inefficient way:
A: StringBuilder sb = new StringBuilder("<Where><And><Neq><FieldRef Name='ID'/><Value Type='Text'>" + Page.Request"id" + " </Value></Neq>");
Should be
StringBuilder sb = new StringBuilder("<Where><And><Neq><FieldRef Name='ID'/><Value Type='Text'>" + Page.Request"id" + " </Value></Neq>");
sb.Append(Page.Request"id");
sb.Append(" </Value></Neq>");

B: sb.Append(string.Format(_condition, arTerms0));
Should be
sb.AppendFormat(_condition, arTerms0);




Developer
Mar 6, 2008 at 12:29 AM
Edited Mar 6, 2008 at 12:29 AM
Marlon,

Thanks for the comments and I'm glad you will use it.

However, I would probably disagree with some of your performance changes as your suggested use of StringBuilder is not the recommended way.

1. Using StringBuilder here would actually be slower, not to mention less readable. The compiler magically turns the concatenation into a String.Concat, which only allocates the string once.

2. string.Format uses a StringBuilder internally and is more readable.

3. Debug statements will get compiled out in release and so no overhead. Trace statements are there so that problems can be resolved in a live environment...they are only used in exception and so cause no overhead in normal operation. Wrapping them in #if DEBUG would negate the reason for having Trace.Write()...they may as well be Debug.Write().

4. StringBuilder would not be better as this is essentially string.Concat(sQuery, sKey, "=", context.Request.QueryStringsKey, "&"). I guess you are looking at EBE Beta2 as I can't find that 'if' statement anymore.

5. I wouldn't necessarily agree that this is bad, but as we are already using a StringBuilder then you could make a case for it...I would probably say it should be using string.Format(). However, looking at the code it probably could be optimized a bit more, but with a loss in readability.

--Vince