RWE: Sample Code Review

   —   
I have got a chance to do code review for one of our client's recently and I thought I would share results with you. You should learn a bit more about our API and how it influences overall website performance. I would also like to use this sample code review to demonstrate what we can do as part of Kentico Professional Services.

What was an issue?

The customer reached us because their custom code (below) was performing very poorly. The client actually reported that single document insert action takes anywhere from 6 to 10 seconds. You would probably agree there must be something wrong with the code as 10 seconds for simple insert is just too much. And you are right - it is very suspicious whenever single action takes so long.

The custom code I attached below imports a new content into Kentico CMS using our API. Source data are retrieved through the channel to external webservice. Perhaps the performance loss can be caused by the delay in the response coming from the webservice. That's true; you cannot really rely on external service. There is no guarantee service would be available all the time and there will always be the factor of hand-shake and request-response cost. Anyway, for this particular case let's assume the delay is no longer than 1 to 3 seconds. We are therefore still looking at additional 7-9 seconds of the execution time lost somewhere along the lines of our custom code.

Let's take closer look on the code now.

How does code look like?

In my analysis I am going to refer to the custom code below. Please review it before you move forward. Please note that for obvious reasons I replaced name of variables and certain lines of code that could suggest who the customer is.


Code Snippet
  1. namespace CUSTOM.Business
  2. {
  3.     /// <summary>
  4.     ///
  5.     /// </summary>
  6.     public static class LocationManager
  7.     {
  8.         private static LocationManagerDataSet _lmds;
  9.         private static BranchManagerDataSet _bmds;
  10.         private static AgeRangeDataSet _agds;
  11.         private static WebTimeCrossReferenceManagerDataSet _wtcds;
  12.         private static WebTimeSFYPManagerDataSet _wtsds;
  13.         private static WebTimeItemCrossReferenceManagerDataSet _wtixds;
  14.         private static JobScheduleDataSet _jsds;
  15.  
  16.         private static DataSet ClassesDataSet;
  17.         private static string durationstr;
  18.         private static string currentSite;
  19.         private static string currentCulture;        
  20.  
  21.         public static string CurrentSite
  22.         {
  23.             get
  24.             {
  25.                 if (string.IsNullOrEmpty(currentSite))
  26.                 {
  27.                     if (CMS.CMSHelper.CMSContext.CurrentSiteName == string.Empty)
  28.                         currentSite = ConfigurationManager.AppSettings["DefaultSite"].ToString();
  29.                     else
  30.                         currentSite = CMS.CMSHelper.CMSContext.CurrentSiteName;
  31.                 }
  32.                 return currentSite;
  33.             }
  34.         }
  35.  
  36.         public static string CurrentCulture
  37.         {
  38.             get
  39.             {
  40.                 if (string.IsNullOrEmpty(currentCulture))
  41.                 {
  42.                     if (CMS.CMSHelper.CMSContext.CurrentSiteName == string.Empty)
  43.                         currentCulture = ConfigurationManager.AppSettings["DefaultCulture"].ToString();
  44.                     else
  45.                         currentCulture = CMS.CMSHelper.CMSContext.CurrentDocumentCulture.CultureCode;
  46.                 }
  47.                 return currentCulture;
  48.             }
  49.         }
  50.  
  51.         public static LocationManagerDataSet GetAllLocations()
  52.         {
  53.             _lmds = SqlDataClient.GetAllLocations();
  54.             return _lmds;
  55.         }
  56.  
  57.         public static JobScheduleDataSet GetAllJobBranches()
  58.         {
  59.             _jsds = SqlDataClient.GetAllJobBranches();
  60.             return _jsds;
  61.         }
  62.  
  63.         public static void UpdateJobBranch(int locationID)
  64.         {
  65.             SqlDataClient.UpdateJobBranch(locationID);
  66.         }
  67.  
  68.  
  69.         public static AgeRangeDataSet GetAllAgeRange()
  70.         {
  71.             _agds = SqlDataClient.GetAllAgeRange();
  72.             return _agds;
  73.         }
  74.         
  75.         public static LocationManagerDataSet GetLocation(int LocationID)
  76.         {
  77.             _lmds = SqlDataClient.GetLocation(LocationID);
  78.             return _lmds;
  79.         }
  80.         
  81.         public static BranchManagerDataSet GetBranchByBranchName(string branchName)
  82.         {
  83.             _bmds = SqlDataClient.GetBranchByBranchName(branchName);
  84.             return _bmds;
  85.         }
  86.         
  87.         public static LocationManagerDataSet GetLocationDataSet(int LocationID)
  88.         {
  89.             _lmds = SqlDataClient.GetLocation(LocationID);
  90.             return _lmds;
  91.         }
  92.  
  93.         public static BranchManagerDataSet GetValidWebTimeBranches()
  94.         {
  95.             _bmds = SqlDataClient.GetValidBranches();
  96.             return _bmds;
  97.         }
  98.  
  99.         public static void GetWebItemItemCrossReferenceByLocationID(int locationID)
  100.         {
  101.             _wtixds = SqlDataClient.GetWebItemItemCrossReferenceByLocationID(locationID);
  102.         }
  103.  
  104.         public static void GetWebTimeCrossReferenceByLocationID(int locationID)
  105.         {
  106.             _wtcds = SqlDataClient.GetWebTimeCrossReferenceByLocationID(locationID);
  107.         }
  108.  
  109.         public static void GetWebTimeSFYPDataByLocationID(int locationID)
  110.         {
  111.             _wtsds = SqlDataClient.GetWebTimeSFYPDataByLocationID(locationID);
  112.         }
  113.  
  114.         public static string RefreshDataFromWebService(int locationID)
  115.         {
  116.             DateTime startTime = DateTime.Now;
  117.             GetLocation(locationID);
  118.  
  119.             Guid batchNumber = Guid.NewGuid();
  120.  
  121.             foreach (LocationManagerDataSet.LocationDataTableRow drow in _lmds.LocationDataTable.Rows)
  122.             {
  123.                 // Create the web request   
  124.                 HttpWebRequest request = WebRequest.Create(string.Format("http://www.somedomain.com", drow.WebserviceLocationID)) as HttpWebRequest;
  125.                 
  126.                 if (request != null)
  127.                 {
  128.                     // Get response   
  129.                     using (HttpWebResponse response = request.GetResponse() as HttpWebResponse)
  130.                     {
  131.                         if (response != null)
  132.                         {
  133.                             string contents = string.Empty;
  134.                             // Load data into a dataset   
  135.                             DataSet dsw = new DataSet();
  136.                             using (StreamReader reader = new StreamReader(response.GetResponseStream()))
  137.                             {
  138.                                 contents = reader.ReadToEnd().Replace("& UP", "+");
  139.                             }
  140.  
  141.                             System.IO.StringReader strcontents = new System.IO.StringReader(contents);
  142.                             WebTimeDataManagerDataSet dsWeather = new WebTimeDataManagerDataSet();
  143.                             dsw.ReadXml(strcontents);
  144.  
  145.                             if (dsw.Tables["Transaction"].Rows[0]["TRAN_STATUS"].ToString() != "FAIL")
  146.                             {
  147.                                 // save the data
  148.                                 SqlDataClient.SaveWebTimeData(dsw, drow.WebtimeLocationID, batchNumber);
  149.                             }
  150.                         }
  151.                     }
  152.                 }
  153.             }
  154.  
  155.             DateTime stopTime = DateTime.Now;
  156.             TimeSpan duration = stopTime - startTime;
  157.             durationstr = duration.ToString();
  158.             return ("RefreshTimes: " + durationstr);
  159.         }
  160.  
  161.         public static string PopulateNodes(int locationID)
  162.         {
  163.             durationstr = string.Empty;
  164.  
  165.             GetLocation(locationID);
  166.             GetValidWebTimeBranches();
  167.  
  168.             foreach (LocationManagerDataSet.LocationDataTableRow drow in _lmds.LocationDataTable.Rows)
  169.             {
  170.                 DeleteSchedules(drow.WebtimeLocationID, _bmds);
  171.                 AddNewSchedules(drow.WebtimeLocationID, _bmds);
  172.             }
  173.  
  174.             return (durationstr);
  175.         }
  176.  
  177.         private static void DeleteSchedules(int locationID, BranchManagerDataSet bm)
  178.         {
  179.             DateTime startTime = DateTime.Now;
  180.             try
  181.             {
  182.                 // look for the Programs Nodes
  183.                 DataRow[] drow = bm.something_Branch.Select("WebtimeLocationID=" + locationID.ToString());
  184.                 string programpath = string.Empty;
  185.                 string unprocessedpath = string.Empty;
  186.  
  187.                 if (drow.Count() > 0)
  188.                 {
  189.                     programpath = ((BranchManagerDataSet.something_BranchRow)drow[0]).NodeAliasPath.ToString() + "/Programs/%";
  190.                 }
  191.  
  192.                 // if there is a real program path, go to the path using the Kentico API
  193.                 if (programpath != string.Empty)
  194.                 {
  195.                     // create a TreeProvider instance
  196.                     UserInfo ui = UserInfoProvider.GetUserInfo("administrator");
  197.                     CMS.TreeEngine.TreeProvider tree = new CMS.TreeEngine.TreeProvider(ui);
  198.  
  199.                     DataSet treeDS = tree.SelectNodes(CurrentSite, programpath, CurrentCulture, true, "something.ClassTime");
  200.                     if (treeDS != null && treeDS.Tables.Count > 0)
  201.                     {
  202.                         CMS.CMSHelper.CMSContext.Init();
  203.                         DataTable dtable = treeDS.Tables[0];
  204.                         foreach (DataRow dr in dtable.Rows)
  205.                         {
  206.                             // Get document with specified site, aliaspath and culture
  207.  
  208.                             CMS.TreeEngine.TreeNode node = tree.SelectSingleNode(CurrentSite, dr["NodeAliaspath"].ToString(), CurrentCulture, true, "something.ClassTime");
  209.                             if (node != null)
  210.                             {
  211.                                 // Delete the document
  212.                                 DocumentHelper.DeleteDocument(node, tree, true, true, true);
  213.                                 node.Delete();
  214.                             }
  215.                         }
  216.                     }
  217.  
  218.                     // delete also from the Unprocessed Bucket
  219.                     unprocessedpath = "/Unprocessed" + ((BranchManagerDataSet.something_BranchRow)drow[0]).NodeAliasPath + "/%";
  220.                     DataSet unprocessedDS = new DataSet();
  221.  
  222.                     unprocessedDS = tree.SelectNodes(CurrentSite, unprocessedpath, CurrentCulture, true, "something.ClassTime");
  223.                     if (unprocessedDS != null && unprocessedDS.Tables.Count > 0)
  224.                     {
  225.                         // this is needed for the windows service routine because it is not in context of the website
  226.                         CMS.CMSHelper.CMSContext.Init();
  227.                         DataTable unprocesseddtable = unprocessedDS.Tables[0];
  228.                         foreach (DataRow dr in unprocesseddtable.Rows)
  229.                         {
  230.                             // Get document with specified site, aliaspath and culture
  231.  
  232.                             CMS.TreeEngine.TreeNode node = tree.SelectSingleNode(CurrentSite, dr["NodeAliaspath"].ToString(), CurrentCulture, true, "something.ClassTime");
  233.                             if (node != null)
  234.                             {
  235.                                 // Delete the document
  236.                                 DocumentHelper.DeleteDocument(node, tree, true, true, true);
  237.                                 node.Delete();
  238.                             }
  239.                         }
  240.                     }
  241.                 }
  242.  
  243.                 DateTime stopTime = DateTime.Now;
  244.                 TimeSpan duration = stopTime - startTime;
  245.                 durationstr = "Delete Times: " + duration.ToString();
  246.             }
  247.             catch (Exception ex)
  248.             {
  249.                 throw new Exception(ex.Message);
  250.             }
  251.         }        
  252.  
  253.         public static CMSTreeNodeOfferingCategoryManagerDataSet GetOfferingCategoryNodesByBranchPath(string branchnamepath)
  254.         {
  255.             CMSTreeNodeOfferingCategoryManagerDataSet currentProgramsDS = new CMSTreeNodeOfferingCategoryManagerDataSet();
  256.  
  257.             UserInfo ui = UserInfoProvider.GetUserInfo("administrator");
  258.             CMS.TreeEngine.TreeProvider tree = new CMS.TreeEngine.TreeProvider(ui);
  259.  
  260.             currentProgramsDS.CMSTreeNodeOfferingCategory.TableName = "something.OfferingCategoryPage";
  261.  
  262.             string programpath = branchnamepath + "/Programs/%";
  263.  
  264.             // get all the program offerings for this program path
  265.             currentProgramsDS.Merge(tree.SelectNodes(CurrentSite, programpath, CurrentCulture, true, "something.OfferingCategoryPage"));
  266.  
  267.             return currentProgramsDS;
  268.         }
  269.  
  270.         public static CMSTreeNodeOfferingCategoryManagerDataSet GetOfferingCategoryNodes(string branchpath)
  271.         {
  272.             CMSTreeNodeOfferingCategoryManagerDataSet currentProgramsDS = new CMSTreeNodeOfferingCategoryManagerDataSet();
  273.  
  274.             UserInfo ui = UserInfoProvider.GetUserInfo("administrator");
  275.             CMS.TreeEngine.TreeProvider tree = new CMS.TreeEngine.TreeProvider(ui);
  276.  
  277.             currentProgramsDS.CMSTreeNodeOfferingCategory.TableName = "something.OfferingCategoryPage";
  278.  
  279.             string programpath = branchpath + "/Programs/%";
  280.  
  281.             // get all the program offerings for this program path
  282.             currentProgramsDS.Merge(tree.SelectNodes(CurrentSite, programpath, CurrentCulture, true, "something.OfferingCategoryPage"));
  283.  
  284.             return currentProgramsDS;
  285.         }        
  286.  
  287.         private static void AddNewSchedules(int locationID, BranchManagerDataSet bm)
  288.         {
  289.             // look for the Programs Nodes
  290.             DataRow[] drow = bm.something_Branch.Select("WebtimeLocationID=" + locationID.ToString());
  291.             string programpath = string.Empty;
  292.  
  293.             DateTime startTime = DateTime.Now;
  294.  
  295.             try
  296.             {
  297.                 if (drow.Count() > 0)
  298.                 {
  299.                     programpath = ((BranchManagerDataSet.something_BranchRow)drow[0]).NodeAliasPath + "/Programs/%";
  300.  
  301.                     // Get the Import Data
  302.                     GetWebTimeSFYPDataByLocationID(locationID);
  303.                     GetWebItemItemCrossReferenceByLocationID(locationID);
  304.  
  305.                     UserInfo ui = UserInfoProvider.GetUserInfo("administrator");
  306.                     CMS.TreeEngine.TreeProvider tree = new CMS.TreeEngine.TreeProvider(ui);
  307.  
  308.                     CMSTreeNodeClassTypeDataSet ClassesDS = new CMSTreeNodeClassTypeDataSet();
  309.                     ClassesDS.CMSTreeNodeClassType.TableName = "something.Class";
  310.  
  311.                     // get all the classes for this program path
  312.  
  313.                     DataSet ds = tree.SelectNodes(CurrentSite, programpath, CurrentCulture, true, "something.Class");
  314.                     ClassesDS.Merge(ds);
  315.  
  316.                     //Read the Import Data First
  317.                     foreach (WebTimeSFYPManagerDataSet.SOMETHING_WebTimeSFYPDataTableRow dsrow in _wtsds.SOMETHING_WebTimeSFYPDataTable.Rows)
  318.                     {                        
  319.                         // locate for the pgm code in the crossreference file
  320.                         string pgmcodetosearch = string.Empty;
  321.  
  322.                         // check if the last 2 are numbers
  323.                         if (dsrow.THRUSTCODE != string.Empty)
  324.                         {
  325.                             // create exception
  326.                             // look for the Programs Nodes
  327.                             string selectstring = "WebTime_ThrustCode='" + dsrow.THRUSTCODE.ToString() + "'";
  328.                             DataRow[] xrows = ClassesDS.Tables[0].Select(selectstring);
  329.  
  330.                             if (xrows.Count() > 0)
  331.                             {
  332.                                 // get parent node for new document
  333.                                 CreateNode(((CMSTreeNodeClassTypeDataSet.CMSTreeNodeClassTypeRow)xrows[0]).NodeAliasPath, tree, dsrow, ((BranchManagerDataSet.something_BranchRow)drow[0]).WebserviceLocationID.ToString());
  334.                             }
  335.                             else
  336.                             {
  337.                                 // create exception
  338.                                 // look for the Programs Nodes
  339.                                 string parentpath = string.Empty;
  340.  
  341.                                 if (drow.Count() > 0)
  342.                                 {
  343.                                     parentpath = "/Unprocessed" + ((BranchManagerDataSet.something_BranchRow)drow[0]).NodeAliasPath;
  344.                                 }
  345.  
  346.                                 CreateNode(parentpath, tree, dsrow, ((BranchManagerDataSet.something_BranchRow)drow[0]).WebserviceLocationID.ToString());
  347.                             }
  348.                         }
  349.                         else
  350.                         {
  351.                             // create exception
  352.                             // look for the Programs Nodes
  353.                             string parentpath = string.Empty;
  354.  
  355.                             if (drow.Count() > 0)
  356.                             {
  357.                                 parentpath = "/Unprocessed" + ((BranchManagerDataSet.something_BranchRow)drow[0]).NodeAliasPath;
  358.                             }
  359.  
  360.                             CreateNode(parentpath, tree, dsrow, ((BranchManagerDataSet.something_BranchRow)drow[0]).WebserviceLocationID.ToString());
  361.                         }
  362.                     }
  363.                 }
  364.  
  365.                 DateTime stopTime = DateTime.Now;
  366.                 TimeSpan duration = stopTime - startTime;
  367.  
  368.                 durationstr = durationstr + "<br/>" + "Add Times: " + duration.ToString() + "<br>";
  369.             }
  370.             catch (Exception ex)
  371.             {
  372.                 throw new Exception(ex.Message);
  373.             }
  374.         }
  375.  
  376.         private static void CreateNode(string parentpath, CMS.TreeEngine.TreeProvider tree, WebTimeSFYPManagerDataSet.SOMETHING_WebTimeSFYPDataTableRow dsrow, string webservicelocationID)
  377.         {
  378.             // get parent node for new document
  379.  
  380.             // this is needed for the windows service because it is not in the context of the website    
  381.             CMS.CMSHelper.CMSContext.Init();
  382.             CMS.TreeEngine.TreeNode parentClassNode = tree.SelectSingleNode(CurrentSite, parentpath, CurrentCulture);
  383.  
  384.             // create a new tree node
  385.             CMS.TreeEngine.TreeNode node = new CMS.TreeEngine.TreeNode("something.ClassTime", tree);
  386.  
  387.             // check off the properties
  388.             if (parentClassNode != null)
  389.             {
  390.                 // set document properties
  391.                 string nodename = GenerateNodeName(dsrow);
  392.                 if (nodename == string.Empty)
  393.                     nodename = dsrow.KEY;
  394.  
  395.                 node.NodeName = nodename;
  396.                 node.NodeAlias = nodename;
  397.  
  398.                 node.SetValue("DocumentCulture", CurrentCulture);
  399.                 node.SetValue("DayOfWeek", nodename);
  400.                 node.SetValue("DocumentName", nodename);
  401.  
  402.                 string startdate = string.Empty;
  403.                 string enddate = string.Empty;
  404.  
  405.                 if (dsrow.BEGIN_DATE != string.Empty)
  406.                 {
  407.                     startdate = DateTime.Now.Year.ToString().Substring(0, 2) + dsrow.BEGIN_DATE;
  408.                     node.SetValue("StartDate", Convert.ToDateTime(startdate.Substring(0, 4) + "/" + startdate.Substring(4, 2) + "/" + startdate.Substring(6, 2)));
  409.                 }
  410.  
  411.                 if (dsrow.END_DATE != string.Empty)
  412.                 {
  413.                     enddate = DateTime.Now.Year.ToString().Substring(0, 2) + dsrow.END_DATE;
  414.                     node.SetValue("EndDate", Convert.ToDateTime(enddate.Substring(0, 4) + "/" + enddate.Substring(4, 2) + "/" + enddate.Substring(6, 2)));
  415.                 }
  416.  
  417.                 node.SetValue("StartTime", dsrow.START_TIME);
  418.                 node.SetValue("EndTime", dsrow.END_TIME); 
  419.                 node.Insert(parentClassNode.NodeID);
  420.  
  421.                 WorkflowManager wm = new WorkflowManager(tree);
  422.  
  423.                 //check if the node supports workflow
  424.                 WorkflowInfo wi = wm.GetNodeWorkflow(node);
  425.                 if (wi != null)
  426.                 {
  427.                     // Approve until the step is publish
  428.                     WorkflowStepInfo currentStep = wm.GetStepInfo(node);
  429.                     if (wm != null)
  430.                     {
  431.                         while ((currentStep != null) && (currentStep.StepName.ToLower() != "published"))
  432.                         {
  433.                             currentStep = wm.MoveToNextStep(node, "");
  434.                         }
  435.                     }
  436.                 }
  437.             }
  438.         }
  439.     }
  440. }


How to improve existing code?

Problem 1: Performing Unnecessary Actions

Let's focus on the PopulateNodes(int locationID) method first. As you can see for every location (basically row record) in the _lmds.LocationDataTable.Rows collection the custom code calls DeleteSchedules(int locationID, BranchManagerDataSet bm) method (line no. 187). Inside this method our code retrieves the collection of 'Programs' nodes existing in the content tree by using the TreeProvide.SelectNodes() method (line no. 216).

As the next step code loops through the retrieved program nodes and for every node it:

a) Gets the NodeAliasPath available in the node data row (line no. 225),

b) Query the database to retrieve the TreeNode object using this NodeAliasPath for the TreeProvider.SelectSingleNode() method (line no. 225),
 
FIX 1: There is actually no reason to call the SelectSingleNode() method to get TreeNode object. All the information about the node is already present in the DataRow dr (line no. 221). Thereby, instead of calling the SelectSingleNode() we can create new instance of the TreeNode object using the new TreeNode(dr, "<document type name>", treeProvider) constructor where we simply pass the DataRow as parameter.
 
The small change suggested above saves 1 query per every program node per every location. If there would be 100 locations with let's say only 10 program nodes per location we would have just saved 1000 quite expensive queries.

Problem 2: Duplicate Actions 

There is another issue with the DeleteSchedules(int locationID, BranchManagerDataSet bm) method. You can see code calls the DocumentHelper.DeleteDocument() method first and then the node.Delete() method right after that (lines no. 229 and 230). The DocumentHelper.DeleteDocument() should be used for documents under the workflow. Hence it is absolutely correct to call DeleteDocument() as long as we are dealing with versioned documents. The node.Delete() method on the other hand should be used only for documents without workflow because unlike DocumentHelper.DeleteDocument() the node.Delete() does not care about multiple versions of the document and therefore is not suitable in our scenario.

FIX 2: There is no reason to call both the DocumentHelper.DeleteDocument() and node.Delete() method at the same place. Actually, it is highly inefficient because the node.Delete() executes additional query for every program node for every location after the document is already wiped off by DeleteDocument() called earlier.

We can again save hundreds or even thousands of queries just by removing the node.Delete() call.

NOTE: The same fix needs to be applied to the code few lines later when working through bucket of unprocessed documents (lines no. 235 - 258).

Problem 3: No Data Caching & Inefficient Insert Operation

Inside the AddNewSchedules(int locationID, BranchManagerDataSet bm) method custom code calls the GetWebTimeSFYPDataByLocationID(int locationID) and GetWebItemItemCrossReferenceByLocationID(int locationID) methods (lines no. 319 and 320) that retrieve data from database executing 2 queries in total for every location. Further in the same method code also retrieves all the location's program nodes (line no. 330). The code then loops through the import data (source data for new document sent from external service) and calls the CreateNode(string parentpath, CMS.TreeEngine.TreeProvider tree, WebTimeSFYPManagerDataSet.SOMETHING_WebTimeSFYPDataTableRow dsrow, string webservicelocationID) method to insert new document in the content tree (lines no. 350, 363 and 377).

In the CreateNode() method custom code uses TreeProvider.SelectSingleNode() (line no. 399) to get the parent node required whenever we are inserting a new document. The CreateNode() method accepts parent NodeAliasPath as first parameter. This parent NodeAliasPath comes from the DataRow that contains full information on the parent node - like NodeID for example.

At the bottom of the CreateNode() method code uses the node.Insert(parentNodeID) (line no. 436) to finally insert a new document in the content tree.

FIX 3: As you can see most of the custom methods (lines no. 68 - 129) pulling out additional information related to the document created (e.g. location details) execute single query for every location being processed. The results of those queries are not cached at all even though caching is perfect fit in scenarios like this. We should therefore implement caching mechanism for the location data obtained from database to streamline data retrieval and processing. The new API used to simplify caching implementations is explained here.

Ideally we should go even step further and instead of querying DB for every location separately we could retrieve details for all locations within the single request, cache the whole result set and later filter out records from the DataSet stored in a memory rather than accessing DB over and over again.

FIX 4: : Instead of selecting the parent node object using the TreeProvider.SelectSingleNode() (line no. 399) using the parent NodeAliasPath we want to pass the parent NodeID as input parameter for the CreateNode() method. The parent NodeID is available in the same DataRow as NodeAliasPath in the original code. This would save us another call to the database curently fired by TreeProvider.SelectSingleNode() (line no. 399). If you realize that every call to the SelectSingleNode() executes additional query to get parent information already available we can eliminate another query for every document created.

FIX 5: To further optimize the code inserting new documents we want to call the node.Insert(parentNodeObj) instead of node.Insert(parentNodeID) (line no. 436). If you call the node.Insert() method with the parent NodeID as parameter the Insert() method will still internally call the SelectSingleNode() to get the parent node object. Let's call the Insert() with the parent node object directly and we get rid of another expensive query. As I have already mentioned to create new instance of parent TreeNode object we just need to use the new TreeNode() constructor and pass the parent DataRow as one of its parameters.
 
FIX 6: In the  CreateNode() method we should use the DocumentHelper.InsertDocument() instead of the node.Insert(). The DocumentHelper.InsertDocument() recognizes whether the node uses workflow or not and performs not only insert of the document itself but also takes care about any additional actions required to properly handle versioning and other workflow related functionality.

Summary

As you could see when it comes to the performance issues with your custom code is more-or-less about efficiency of your code. And by most part the efficiency goes hand in hand with costs of data access and transfer. That being said your attention should be aimed on how much data you retrieve from data base, what is reusability level of such data and whether or not the same data are not already available at different source.

Before you go...

I would love to hear some feedback from you on whether you find posts like this useful, what I should change or add to better serve their purpose and so on. I can imagine we could do some series from articles on code review/best practices topics, however, I need to know if it is something you may be interested in. Please use comments below to pass the message.

Thank you!

K.J.
Share this article on   LinkedIn Google+

Karol Jarkovsky

Director of Product

Comments

Marc commented on

Thank you for spending time in this! This is a good topic to save up time in developing similiar proceses. Very usefull example!

It's nice to see that almost same methods could make that big differences.

Armysniper89 commented on

Very useful...Kentico has so many ways to do things that it is important for us as developers to understand the right way to approach the code. As an API guy, this is very important to understand and see how others are approaching the API to solve problems.

henry Chong commented on

Great read :) look forward to more optimization posts in the future!