I'm currently supporting an ASP.NET project that has a lot of code like this using the System.Web.UI.Control.FindControl method to get data from child controls - in this example, from DataListItems belonging to a DataList control:
foreach (DataListItem item in employeeDataList.Items)
{
if (item.ItemType == ListItemType.Item ||
item.ItemType == ListItemType.AlternatingItem)
{
if (((HtmlInputCheckBox)item.FindControl("SelectionCheckBox")).Checked)
{
string positionId = ((Label)item.FindControl("PositionIdLabel")).Text;
string personnelId = ((Label)item.FindControl("PersonnelIdLabel")).Text;
ProcessPosition(positionId, personnelId);
}
}
}
There's much room for improvement, but I started by adding a couple of FindControl overrides to the base class inherited by the project's pages.
The first calls the passed "parent" control's FindControl method, and throws an System.ArgumentOutOfRangeException if the specified ID is not found. This way, instead of just failing when later trying to use a null object, we know exactly which ID wasn't found in which "parent" control:
protected Control FindControl(Control parentControl, string id)
{
Control control = parentControl.FindControl(id);
if (control == null)
{
throw new ArgumentOutOfRangeException(
string.Format("{0}.FindControl failed for ID \"{1}\"",
parentControl.ID, id));
}
return control;
}
The second uses generics to call the override shown above and cast the found Control to the specified Control type (the "where T : Control " constraint allows it to return only types that inherit from System.Web.UI.Control).
protected T FindControl<T>(Control parentControl, string id) where T : Control
{
return (T)FindControl(parentControl, id);
}
Here's the code shown in the first example, modified to use the generic FindControl method. I think it's a little more readable than the original:
foreach (DataListItem item in employeeDataList.Items)
{
if (item.ItemType == ListItemType.Item ||
item.ItemType == ListItemType.AlternatingItem)
{
if (FindControl<HtmlInputCheckBox>(item, "SelectionCheckBox").Checked)
{
string positionId = FindControl<Label>(item, "PositionIdLabel").Text;
string personnelId = FindControl<Label>(item, "PersonnelIdLabel").Text;
ProcessPosition(positionId, personnelId);
}
}
}