C# How to refactor my function code which speed up my function execution time

Question

See the first block of code. here i am doing cross join between a ** list and datatable** from where i am calling a function called ExtractStandardValueFromLinkText this function has many loop which take times to complete.

tell me how could i refactor the code in 2nd block to speed up the execution.

all element start with dt means datatable object and all lst means list object. i use AsParallel for querying datatable which speed up execution bit but i am looking for very good execution speed. so what way i can refactor 2nd block code which speed up the execution. if any area is not clear then ask me. so i will explain more. please share idea how best way to refactor 2nd block code.

1st block of code

 if (dtAccumulatedData.AsEnumerable().AsParallel().Any(x => !string.IsNullOrEmpty(x.Field<string>("LinkedItemList"))
 && x.Field<string>("Link").ToUpper() == "TRUE"))
 {
 DataTable dtFilterForLinkOnlyTrue = dtAccumulatedData.AsEnumerable().AsParallel()
 .Where(x => !string.IsNullOrEmpty(x.Field<string>("LinkedItemList"))
 && x.Field<string>("Link").ToUpper() == "TRUE")
 .OrderByDescending(a => a.Field<string>("Matched Section"))
 .ThenBy(a => a.Field<string>("Matched Items"))
 .ThenByDescending(a => a.Field<string>("Link"))
 .CopyToDataTable();
 //iterate in datatable of dtFilterForLink where LinkedItemList is not empty
 if (dtFilterForLinkOnlyTrue != null && dtFilterForLinkOnlyTrue.Rows.Count > 0)
 {
 var crossjoindata = (from _dtFilterForLinkOnlyTrue in dtFilterForLinkOnlyTrue.AsEnumerable()
 from p in _Periods
 select new BrokerData
 {
 StandardValue = ExtractStandardValueFromLinkText(
 (_dtFilterForLinkOnlyTrue.Field<string>("Matched Section") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("Matched Section").ToString()),
 (_dtFilterForLinkOnlyTrue.Field<string>("Matched Items") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("Matched Items").ToString()),
 p.ToString(),
 (_dtFilterForLinkOnlyTrue.Field<string>("Tab") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("Tab").ToString()),
 (_dtFilterForLinkOnlyTrue.Field<string>("Row") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("Row").ToString()),
 (_dtFilterForLinkOnlyTrue.Field<string>("Units") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("Units").ToString()),
 ref dtAccumulatedPeiodicalData,
 (_dtFilterForLinkOnlyTrue.Field<string>("LinkedItemList") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("LinkedItemList").ToString()),
 (_dtFilterForLinkOnlyTrue.Field<string>("Broker Items") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("Broker Items").ToString()),
 (_dtFilterForLinkOnlyTrue.Field<string>("Action") == null ? "" : _dtFilterForLinkOnlyTrue.Field<string>("Action").ToString()),
 (_dtFilterForLinkOnlyTrue.Field<string>("cumulative") == null ? false :
 (_dtFilterForLinkOnlyTrue.Field<string>("cumulative").ToString().ToUpper() == "TRUE" ? true : false)))
 }).ToList();
 }
 }
 lstDataCheck = null;

2nd block of code

private string ExtractStandardValueFromLinkText(string TabName, string StandardLineItem, string strPeriod, string BRTab, string RowNumber, string strUnits,
 ref DataTable dtFilterDataFromAllData, string LinkedItemList, string BRLineItem, string strAction, bool cumulative) 
 {
 string[] childformulas = null;
 string _strBRTab = "", _RowNumber = "";
 try
 {
 
 //this line prevent to consider same tabname & Lineitem in for loop
 if (!lstDataCheck.Contains(BRTab + "~" + RowNumber + "~" + strPeriod))
 {
 //adding tabname & Lineitem in a list if tabname & Lineitem not found in lstDataCheck list
 //lstDataCheck.Add(TabName + "~" + StandardLineItem + "~" + strPeriod);
 lstDataCheck.Add(BRTab + "~" + RowNumber + "~" + strPeriod);
 //this StandardValue will hold parent StandardValue value
 StandardValue = null;
 if (BRTab != "")
 {
 //checking parent records has StandardValue or not
 if (dtFilterDataFromAllData.AsEnumerable().AsParallel().Any(w => w.Field<string>("BRTab") == BRTab
 && w.Field<string>("RowNumber") == RowNumber
 && w.Field<bool>("Link").ToString().ToUpper() == "TRUE"
 && w.Field<string>("StandardDate").Replace("A", "").Replace("E", "") == strPeriod.Replace("A", "").Replace("E", "")))
 {
 //storing parent StandardValue
 StandardValue = dtFilterDataFromAllData.AsEnumerable().AsParallel()
 .Where(w => w.Field<string>("BRTab") == BRTab
 && w.Field<string>("RowNumber") == RowNumber
 && w.Field<bool>("Link").ToString().ToUpper() == "TRUE"
 && w.Field<string>("StandardDate").Replace("A", "").Replace("E", "") == strPeriod.Replace("A", "").Replace("E", ""))
 .Select(v => v.Field<string>("StandardValue"))
 .FirstOrDefault();
 }
 }
 else
 {
 if (dtFilterDataFromAllData.AsEnumerable().AsParallel().Any(w => w.Field<string>("TabName") == TabName
 && w.Field<string>("StandardLineItem") == StandardLineItem
 && w.Field<bool>("Link").ToString().ToUpper() == "TRUE"
 && w.Field<string>("StandardDate").Replace("A", "").Replace("E", "") == strPeriod.Replace("A", "").Replace("E", "")))
 {
 //storing parent StandardValue
 StandardValue = dtFilterDataFromAllData.AsEnumerable().Where(w => w.Field<string>("TabName") == TabName
 && w.Field<string>("StandardLineItem") == StandardLineItem
 && w.Field<bool>("Link").ToString().ToUpper() == "TRUE"
 && w.Field<string>("StandardDate").Replace("A", "").Replace("E", "") == strPeriod.Replace("A", "").Replace("E", ""))
 .Select(v => v.Field<string>("StandardValue"))
 .FirstOrDefault();
 }
 }
 //When parent StandardValue is null or empty
 if (StandardValue == null || StandardValue.ToString() == "")
 {
 if (LinkedItemList.Trim() != "")
 {
 if (LinkedItemList.StartsWith("~"))
 {
 LinkedItemList = LinkedItemList.Substring(1, LinkedItemList.Length - 1);
 }
 //split link text by ~sign
 childformulas = LinkedItemList.Split('~');
 foreach (string cf in childformulas)
 {
 if (cf != "")
 {
 //iterate in linktext collection
 _strBRTab = cf.Split('`')[1].ToString().Trim();
 _RowNumber = cf.Split('`')[2].ToString().Trim();
 //querying main resultset to check child records has data
 ChildStandardValue = dtFilterDataFromAllData.AsEnumerable().AsParallel()
 .Where(w => w.Field<string>("BRTab") == _strBRTab
 && w.Field<string>("RowNumber") == _RowNumber
 && w.Field<string>("StandardDate").Replace("A", "").Replace("E", "") == strPeriod.Replace("A", "").Replace("E", "")
 && w.Field<bool>("Link").ToString().ToUpper() == "FALSE")
 .Select(v => v.Field<string>("StandardValue"))
 .FirstOrDefault();
 if (ChildStandardValue != null && ChildStandardValue.ToString() != "")
 {
 _strBRTab = string.Empty;
 _RowNumber = string.Empty;
 break;
 }
 _strBRTab = string.Empty;
 _RowNumber = string.Empty;
 }
 _strBRTab = string.Empty;
 _RowNumber = string.Empty;
 }
 if (ChildStandardValue != null && ChildStandardValue != "")
 {
 if (StandardValue == null) //when parent Standard Value not found means null
 {
 //here inserting new row in data table with Standard Value data
 DataRow dr = dtFilterDataFromAllData.NewRow();
 dr["TabName"] = TabName;
 dr["StandardDate"] = strPeriod;
 dr["BRTab"] = BRTab;
 dr["BRLineItem"] = BRLineItem;
 dr["Action"] = strAction;
 dr["StandardLineItem"] = StandardLineItem;
 dr["StandardValue"] = ChildStandardValue;
 dr["Link"] = "True";
 dr["LinkedItemList"] = LinkedItemList;
 dr["RowNumber"] = RowNumber;
 dr["cumulative"] = cumulative;
 dtFilterDataFromAllData.Rows.Add(dr);
 }
 else if (StandardValue == "") //when Standard Value found but has empty data
 {
 //here updating Standard Value
 if (BRTab != "")
 {
 //BRTab will not be empty when that will be direct mapping with linking
 dtFilterDataFromAllData.AsEnumerable().Where(w => w.Field<string>("BRTab") == BRTab
 && w.Field<string>("RowNumber") == RowNumber
 && w.Field<bool>("Link").ToString().ToUpper() == "TRUE"
 && w.Field<string>("StandardDate").Replace("A", "").Replace("E", "") == strPeriod.Replace("A", "").Replace("E", ""))
 .ForEach(w =>
 {
 //update parent's StandardValue with child StandardValue
 w["StandardValue"] = ChildStandardValue;
 });
 dtFilterDataFromAllData.AcceptChanges();
 }
 else
 {
 //BRTab will be empty when that will be calculated item with linking
 dtFilterDataFromAllData.AsEnumerable().Where(w => w.Field<string>("TabName") == TabName
 && w.Field<string>("StandardLineItem") == StandardLineItem
 && w.Field<bool>("Link").ToString().ToUpper() == "TRUE"
 && w.Field<string>("StandardDate").Replace("A", "").Replace("E", "") == strPeriod.Replace("A", "").Replace("E", ""))
 .ForEach(w =>
 {
 //update parent's StandardValue with child StandardValue
 w["StandardValue"] = ChildStandardValue;
 });
 dtFilterDataFromAllData.AcceptChanges();
 }
 }
 }
 }
 }
 }
 }
 catch(Exception ex)
 {
 }
 return "";
 } 

In the second block of code i am storing standard value and also update a data table dtFilterDataFromAllData that is the reason for which i can not do this whole operation under AsParallel. should i use ForAll instead of foreach to gain some speed ?

looking for guide line and suggestion.

Thanks

• Heslacher: Please state in the question title what you code is supposed to do and explain the code in more detail in the question itself. You should show the whole method as well, otherwise we would have to guess what should happen which leads to misunderstandings.
• user227579: when parent standard value is empty or null then i iterate in foreach and query a data table by LINQ. if value found then some time insert that data into data table or update data table. basically it is taking long time. if you see my 1st block of code where i cross join two list and data table from where second block code fire and in second block there is another foreach loop. including all it is taking long time to complete the execution. so what approach i should follow to minimize the execution time?
• Heslacher: "and explain the code in more detail in the question itself. " not in the comments.
• user227579: ok i will explain the code in more details at night whenever i will be free from office work and let you know. please remove negative marking because this is my first question in this site. negative mark demotivate people. thanks
• Heslacher: I will remove my downvote after you have changed the question and I think it doesn't deserve a downvote anymore. If the question becomes a good question I will place an upvote.

Answer 1

I'm going to do a superficial review of your 2nd block of code from a readability point of view. If I stumble across something that might help performance, I'll point it out but you need clear code first. Other things become much easier when the code is simple. Remember, we optimise code for other developers to read first. Once we've done that, we can do all sorts of tricks to make the code faster but they almost always hamper readability - you should only do it when you have evidence that it will be of material benefit.

Onwards, then.

---

C# uses camelCase for parameters, we like readable names and Hungarian notation is not a good thing. We have a type system, we don't need to add type information to the names.

private string ExtractStandardValueFromLinkText(
 string tabName, 
 string standardLineItem, 
 string period,
 string brTab,
 string rowNumber,
 string unit,
 ref DataTable data,
 string linkedItemList,
 string brLineItem,
 string action,
 bool cumulative) 

Now, I've put each parameter on a new line to highlight another point. There are too many parameters to this method. I'm not going to suggest too much here but it looks you could benefit from some classes to group related things together.

string _strBRTab = "", _RowNumber = "";

Nope. Don't use _ for local variables. Some people use _ to prefix fields so that's doubly confusing.

Let's consider the next line:

 //this line prevent to consider same tabname & Lineitem in for loop
 if (!lstDataCheck.Contains(BRTab + "~" + RowNumber + "~" + strPeriod))
 {

You need a comment to explain what the code does? That is a huge hint that a well-named method is in order.

if (!HasBeenProcessed(brTab, rowNumber, period))
{ 

And then we inverse the if to reduce the nesting:

if (HasBeenProcessed(brTab, rowNumber, period))
{
 return "Already processed";
}

You've just saved a lot of tabs in the rest of your method.

I don't know what type lstDataCheck is but I would guess that it's a list. Make it a HashSet<string> instead to get O(1) lookups.

And again, a comment to explain the code and some bonus commented out code.

//adding tabname & Lineitem in a list if tabname & Lineitem not found in lstDataCheck list
//lstDataCheck.Add(TabName + "~" + StandardLineItem + "~" + strPeriod);
lstDataCheck.Add(BRTab + "~" + RowNumber + "~" + strPeriod);

Commented out code should be deleted.

You can delete your explanation by introducing a method again:

MarkRowAsProcessed(brTab, rowNumber, period);

It all gets a bit too much for me after that. You need more methods and more abstraction to make this readable.

Oh, this is a bad thing:

catch(Exception ex)
{
}
return "";

Your method always returns "". You've used string.Empty in other places so the inconsistency is another stumbling block when reading. Never catch all exceptions and ignore. On the rare case where you can safely do it, add a comment:

// We ignore all failures here because <clear reason why there are no mistakes I need 
// to know about in the above code>. 

But seriously, a method this big and try-catch-ignore? Not good.