Okay, there are two main reasons to refactor. The first is clarity of code. Since we have a working base, we can now concentrate on making our logic readable, breaking major blocks of code into separate methods, as well as possibly change how certain calls work. The second major reason to refactor is for performance or scale. One needs to first realize that scale and performance aren't always the same thing. Scaling is about consistency, where performance is about speed. The two often have the same result, but not always. As an example, using out of process ASP.Net Sessions scales better, but the performance of a single connection is slower.
When refactoring for readability it often comes down to utilizing the framework and, or the language being used. Often there are language constructs or framework objects and methods that can increase clarity.
try
{
if (Context.Items["SomeValue"] != null)
someVar = int.Parse(Context.Items["SomeValue"]);
else
someVar = 0;
}
catch (Exception)
{
someVar = 0;
}
The above example is fairly common in C# (.Net) code. This is because many people don't know about the TryParse methods that were added in .Net version 2.
int.TryParse(Context.Items["SomeValue"].ToString(), someVar);
Just that small piece of knowledge about the framework being used can mean a lot less code. This example doesn't really improve performance, so much as readability. This is because it mearly abstracts your logic out. Just the same, imagine having to validate a dozen numeric inputs in a row.
As to performance, there is another new development in .Net that can help a lot. A project I am working on was using a DataTable in memory as retreived from a database call using a DataAdapter to store said settings in memory. When looking up a particular value the .Select() method of the DataTable was being called with the given search value for the key. This is relatively quick, until you consider that this was being called a few dozen times per page, on a site with many thousands of simultaneous users. Below is the logic for the before.
public static string GetConfigValue(string key)
{
string ret = null;
DataTable dtConfigSettings = GetConfig();
if (key != string.Empty)
{
DataRow[] drs = dtConfigSettings.Select("KeyName='" + key + "'");
if (drs != null && drs.Length != 0)
ret = drs[0]["KeyValue"].ToString();
}
return ret;
}
And here is the logic after...
public static string GetConfigValue(string key)
{
string ret = null;
Dictionary settings = GetConfig();
try {
ret = settings[key.ToString().Trim().ToLower()]; //avoid excessive check/get, rely on exception.
} catch(Exception ex) {
EventLogger.HandleException(ex, "Log Only Policy");
}
return ret;
}
Basically this comes down to, "don't use a dump truck to deliver your mail." The DataTable is a fairly large object, and using the .Select() method a lot more overhead than using what equates to a hash-table index lookup. Now, you may notice that I use a try/catch block in place of checking to see if a given key exists. This is because I expect the key to be there every time, and only need the catch block as a bit of protection. Checking on settings.ContainsKey() would mean the index for the Dictionary would be searched twice. Because this method is called many times, by many users, on many pages, even the near insignificant amount of performance difference in the before and after of this method add up to seconds on the load of various pages.