Help reducing loops with filter code

Question

The following code is very slow. I was wondering if you can help me speed it up.

private void filter(List<CheckBox> boxList)
 {
 // refLanguageTabs is a TabControl the Tabcount is 9
 for (int i = 0; i < refLanguageTabs.TabCount; i++)
 {
 Control[] ctrls = refLanguageTabs.TabPages[i].Controls.Find(refLanguageTabs.TabPages[i].Name + "Grid", true);
 DataGridView dgv = ctrls[0] as DataGridView;
 // average row count is 3000 
 for (int j = 0; j < dgv.RowCount; j++)
 {
 for (int k = 0; k < boxList.Count; k++)
 {
 if (dgv.Rows[j].Cells[1].Value.ToString() != boxList[k].Name.ToString())
 {
 dgv.Rows[j].Visible = false;
 }
 }
 }
 dgv.Refresh();
 }
 }

• Caridorc: Only state the code purpose in the title, what does it do specifically?

Answer 1

Some few thoughts without testing or anything else:

• Cache the value of the cell in a local variable.
• Replace for-loops if possible with foreach-loops.
• The call of boxList[x].Name.ToString() is unnecessary, it is already a String.
• break after setting the visibility to avoid unnecessary checks.

foreach(DataGridViewRow row in dgv.Rows)
{
 String cellValue = row.Cells[1].Value.ToString();
 foreach(CheckBox boxItem in boxList)
 {
 if(cellValue == boxItem.Name)
 {
 row.Visible = false;
 break;
 }
 }
}

On another sidenote, please use meaningful variable names, even in such stupid iterating loops.

• I am not smart: Is there a speed difference between for and foreach?
• Mike Nakis: Yes, there can be. With for() you have to use indexed arrays, and when you use indexed arrays the compiler has to emit code to check that your indexes are within the bounds of your array all the time. With foreach() you do not use indexed arrays, so you save those checks.
• Bobby: @PoiXen: The main reason is readability.
• Marko: @MikeNakis - my understanding is that for() is faster than foreach() despite the index bound checking. If you perform a Google search "for vs foreach performance" you would find that a lot of people would disagree with your statement.
• Mike Nakis: @Marko I did the Google search that you suggested, and what I saw is that the answer to the question is still highly debatable, though most indications appear to suggest that for() will usually be somewhat faster, indeed. So, I stand corrected. But in any case, whatever performance difference there is, it will be negligible as far as the question at hand is concerned. The better readability of the foreach() loop is almost always worth the few extra clock cycles.

Answer 2

My recommendation would be to first make one pass through your cells and populate a dictionary in which the key is the name and the value is the cell itself. Then make a single pass through the checkboxes, and for each checkbox use its name to lookup the cell in the dictionary and there you have it.

• Mike Nakis: Whoever upvoted this, thank you. It is a pity how the best answer is some times overlooked! C-:=
• Bobby: What name do you mean to use for the key?
• Mike Nakis: Hmmmm, I meant the value of the cell. As in MyDictionary[row.Cells[1].Value] = row.Cells[1]
• Bobby: And what if there are multiple cells with the same value?
• Mike Nakis: I did not know that was a possibility, but I suppose I should have thought of it. So, in that case, you can reverse it, assuming that you do not have two checkboxes with the same name. (Which sounds like a reasonable assumption to make.) First enumerate the checkboxes and populate a dictionary mapping checkbox names to checkboxes. Then, enumerate your rows, and look up the corresponding checkbox using the value of the cell. This actually makes more sense, because I assume you have fewer checkboxes than rows, so the dictionary will take up less memory.

Answer 3

If you can stand a little LINQ and Task Parallel library, this might give it a little boost:

 private void filter(IEnumerable<CheckBox> boxList)
 {
 // refLanguageTabs is a TabControl the Tabcount is 9
 for (var i = 0; i < refLanguageTabs.TabCount; i++)
 {
 var ctrls = refLanguageTabs.TabPages[i].Controls.Find(refLanguageTabs.TabPages[i].Name + "Grid", true);
 var dgv = ctrls.Any() ? ctrls[0] as DataGridView : null;
 if (dgv == null)
 {
 continue;
 }
 // average row count is 3000
 Parallel.For(0, dgv.RowCount, j =>
 {
 foreach (var t in boxList.Where(t => dgv.Rows[j].Cells[1].Value.ToString() != t.Name.ToString()))
 {
 dgv.BeginInvoke(new MethodInvoker(() => { dgv.Rows[j].Visible = false; }));
 }
 });
 dgv.Refresh();
 }
 }