How to make my code faster and easier?

Question

This is my .cs part code where I am calling storeprocedure in LINQ to SQL:

var rr_j_cat = db.allcategories().ToList();//its store procedure calling thousands of rows
if (rr_j_cat.Count() != 0)
 {
 DataTable dt = new DataTable();//making dynamic datatable
 dt.Columns.Add("sub_id"); //dynamic columns
 dt.Columns.Add("sibheadername"); //dynamic columns
 foreach (var it in rr_j_cat) // will rotate thousands time which makes process talking more time 
 {
 DataRow dr = dt.NewRow(); // dyanamic rows
 var rr_sel_cat = db.selcategories(Convert.ToInt32(it.sub_id)).ToList(); //another storeprocedure to check a count of data present in another data or table 
 int count = Convert.ToInt32(rr_sel_cat.First().Column1);
 if (count != 0)
 {
 dr["sub_id"] = it.sub_id;
 dr["sibheadername"] = it.sibheadername + " (" + count + ")"; // adding count besides subheader name which you can see in below image on dance 
 count =0;
 }
 else
 {
 dr["sub_id"] = it.sub_id;
 dr["sibheadername"] = it.sibheadername;
 }
 dt.Rows.Add(dr);
 }
 dd_j_area.DataSource = dt; // binding with dropdownlist
 dd_j_area.DataTextField = "sibheadername";
 dd_j_area.DataValueField = "sub_id";
 dd_j_area.DataBind();
 dd_j_area.Items.Insert(0, "");
}

It's working fine, but taking more time.

It will show as

here

• dreza: Can you not have one stored procedure return you the results rather than having to call two?
• Fiddling Bits: Have you thought about writing it in assembly? Bad and overused joke... sorry.

Answer 1

The principal performance issues are on the database side (or maybe you could do some kind of caching or pre-loading), but your code could be made much clearer and shorter (there should be also some minor performance improvements):

string[] columnNames = new[] { "sub_id", "sibheadername" };
var categories = db.allcategories().ToList(); 
if (categories.Count == 0)
{
 return;
}
var dt = new DataTable();
dt.Columns.Add(columnNames[0]);
dt.Columns.Add(columnNames[1]);
foreach (var category in categories)
{
 var selectedCategories = db.selcategories(Convert.ToInt32(category.sub_id)).ToList();
 int count = Convert.ToInt32(selectedCategories.First().Column1);
 DataRow dr = dt.NewRow();
 dr[0] = category.sub_id;
 dr[1] = count != 0
 ? category.sibheadername + " (" + count + ")" // btw. this is FASTER (in this case) than a StringBuilder!
 : category.sibheadername;
 dt.Rows.Add(dr);
}
areaDropdown.DataSource = dt;
areaDropdown.DataTextField = columnNames[1];
areaDropdown.DataValueField = columnNames[0];
areaDropdown.DataBind();
areaDropdown.Items.Insert(0, "");

• Xtremcool: dr is missing..
• Thomas Weller: You're right. Simply overseen it. Will edit the answer.
• Xtremcool: hey its working faster
• Thomas Weller: Nice to hear! You're welcome.
• Xtremcool: As i am using storeprocedure is there any necessary of using precompiling??

Answer 2

This is how it should be using StringBuilder:

 System.Text.StringBuilder sibHeaderNameBuilder = new StringBuilder();
 sibHeaderNameBuilder.Append(it.sibheadername);
 sibHeaderNameBuilder.Append("(");
 sibHeaderNameBuilder.Append(count);
 sibHeaderNameBuilder.Append(")");
 dr["sibheadername"] = sibHeaderNameBuilder.ToString();

Note that using the stringBuilder.Append(int value) method, you don't have to convert use the string representation of count.

• Xtremcool: its adding one more class of using System.Text; ....and on every loop it call this....i think it will make more load on page of name space and this code?????think so...
• Arin Ghazarian: OK, but you should instantiate the stringBuilder object outside of your loop. Also instead of using string concatenation you can use String.Format(), which will be more faster.
• Thomas Weller: Umhh, nope. Using StringBuilder is actually slower in this concrete case, because the compiler will optimize the statement " (" + count + ")".
• Arin Ghazarian: You are right Thomas about the StringBuilder, but I think StringBuilder is a little bit more efficient (in case of memory management) than using string concatenation.
• Thomas Weller: Generally speaking yes. Actually a lot more efficient (I've seen cases where this was around 1000 times faster). The only case where direct concatenation is faster is the "x" + <somevar> + "y" scenario.

Answer 3

You asked two things,

1. Clearer code
2. Better performance

I'll try to cover both.

First of all it's unnecessary to convert rr_j_cat and rr_sel_cat to list again! So rr_j and rr_list variables are both useless.

Second, instead of converting rr_sel_cat.First().Column1 to string, which in your case will instantiate a string variable (count) over and over, convert it to and int or if it's already an int or a value type, use it instead and avoid unnecessary boxing which is a performance issue.

Third, instead of string concatenating in dr["sibheadername"] = it.sibheadername + " (" + count + ")";, use StringBuilder class which will be a good performance improvement in your case.

• Xtremcool: how to make use stringbuilder in this case?..please will u give some demo based on my question??
• Xtremcool: as you suggested other things are being changed....but string builder is good to use ?
• Arin Ghazarian: Actually StringBuilder is more efficient in case of using memory, but the fastest way will be using string.Format().
• Thomas Weller: Umhh, nope. First, the ToList() prevents the stored procedure from being executed more than once. Second, using StringBuilder is actually slower in this concrete case, because the compiler will optimize the statement " (" + count + ")".