What will be optimized version of the given code?
I have a block of code where i am checking checkboxes, comparing the contents of the datatable in the given code:
foreach (DataRow dr in dtResult.Rows)
{
for (var i = 0; i < chkboxListWorkTypes.Items.Count; i++)
{
if (chkboxListWorkTypes.Items[i].Value.Equals(dr["WorkTypeID"].ToString()))
{
chkboxListWorkTypes.It开发者_开发百科ems[i].Selected = true;
}
}
}
Any labmda or linq expression would be great.
Real optimization will be a data binding.
This is what I do
foreach (DataRow dr in dtResult.Rows)
{
string cWorkTypeID = dr["WorkTypeID"].ToString();
for (var i = 0; i < chkboxListWorkTypes.Items.Count; i++)
{
if(chkboxListWorkTypes.Items[i].Value.Equals(cWorkTypeID))
chkboxListWorkTypes.Items[i].Selected =true;
}
}
if the ID on the checkboxes is diferent (that probably is),
foreach (DataRow dr in dtResult.Rows)
{
string cWorkTypeID = dr["WorkTypeID"].ToString();
for (var i = 0; i < chkboxListWorkTypes.Items.Count; i++)
{
if(chkboxListWorkTypes.Items[i].Value.Equals(cWorkTypeID))
{
chkboxListWorkTypes.Items[i].Selected = true;
break;
}
}
}
Though it does practically the same thing behind the scenes as @Aristos answer, doing it like the following will cut your lines of code down a bit:
foreach (DataRow dr in dtResult.Rows)
{
ListItem item = chkboxListWorkTypes.Items.FindByValue(dr["WorkTypeID"].ToString());
if (item != null)
{
item.Selected = true;
}
}
You're essentially performing a JOIN operation. There are 3 types of join algorithms in common use - nested loop, hash, and merge. Your code is using the nested loop algorithm - which seems appropriate for table sizes of 10 and 5, unless one or both are already sorted (in which case, a merge join may be more appropriate).
I can't imagine this being a bottleneck in any real-world application - but, we can possibly improve it a little bit with some assumptions.
- Let's start by hoisting variables, to ensure they're not being recalculated (the JIT-optimizer will probably do this automatically, but it doesn't hurt):
int chkBoxCount = chkboxListWorkTypes.Items.Count; foreach (DataRow dr in dtResult.Rows) { string rowValue = dr["WorkTypeID"].ToString(); for (var i = 0; i < chkBoxCount; i++) { var chkBox = chkboxListWorkTypes.Items[i]; if (chkBox.Value.Equals(rowValue)) { chkBox.Selected = true; } } }
- On the assumption that rowValue is a string, let's use a cast:
int chkBoxCount = chkboxListWorkTypes.Items.Count; foreach (DataRow dr in dtResult.Rows) { string rowValue = (string)dr["WorkTypeID"]; for (var i = 0; i < chkBoxCount; i++) { var chkBox = chkboxListWorkTypes.Items[i]; if (chkBox.Value.Equals(rowValue)) { chkBox.Selected = true; } } }
- Since we're never unselecting a checkbox, and a bool compare should be faster on average than a string compare:
int chkBoxCount = chkboxListWorkTypes.Items.Count; foreach (DataRow dr in dtResult.Rows) { string rowValue = (string)dr["WorkTypeID"]; for (var i = 0; i < chkBoxCount; i++) { var chkBox = chkboxListWorkTypes.Items[i]; if (!chkBox.Selected && chkBox.Value.Equals(rowValue)) { chkBox.Selected = true; } } }
- Assuming each checkbox value is unique, we can stop iterating when we've found a result:
int chkBoxCount = chkboxListWorkTypes.Items.Count; foreach (DataRow dr in dtResult.Rows) { string rowValue = (string)dr["WorkTypeID"]; for (var i = 0; i < chkBoxCount; i++) { var chkBox = chkboxListWorkTypes.Items[i]; if (!chkBox.Selected && chkBox.Value.Equals(rowValue)) { chkBox.Selected = true; break; // stop looking } } }
- Since you have 10 rows, and 5 checkboxes - we should reverse the loops. Since we break on the inner loop when value is found, making the inner loop the larger of the 2 is more efficient:
int chkBoxCount = chkboxListWorkTypes.Items.Count; for (var i = 0; i < chkBoxCount; i++) { var chkBox = chkboxListWorkTypes.Items[i]; if (chkBox.Selected) continue; foreach (DataRow dr in dtResult.Rows) { string rowValue = (string)dr["WorkTypeID"]; if (chkBox.Value.Equals(rowValue)) { chkBox.Selected = true; break; } } }
- Assuming you don't care about culture or case sensitive comparisons, we can use OrdinalIgnoreCase:
int chkBoxCount = chkboxListWorkTypes.Items.Count; for (var i = 0; i < chkBoxCount; i++) { var chkBox = chkboxListWorkTypes.Items[i]; if (chkBox.Selected) continue; foreach (DataRow dr in dtResult.Rows) { string rowValue = (string)dr["WorkTypeID"]; if (string.Equals(chkBox.Value, rowValue, StringComparison.OrdinalIgnoreCase)) { chkBox.Selected = true; break; } } }
That's about the best I can do. I'd be surprised if there was any measurable difference on such a small dataset. So, here's what you should really do:
var workTypeIds = dtResult.Rows.Cast<DataRow>().Select(dr => (string)dr["WorkTypeId"]);
foreach (var chk in chkBoxListWorkTypes)
{
if (workTypeIds.Contains(chk.Value))
{
chk.Selected = true;
}
}
Or, with an Each extension:
var workTypeIds = dtResult.Rows.Cast<DataRow>().Select(dr => (string)dr["WorkTypeId"]);
chkBoxListWorkTypes.Where(c => workTypeIds.Contains(c.Value)).Each(c => c.Selected = true);
which may be slightly slower, but reads a lot better IMO.
精彩评论