Bad code alert =)

Haha, today a collegue of mine found a very funny piece of code unfortunatly located in on of our products, behold the 100 repeater-item killer:

The asp.net code-behind code is:

   List list = null;
   foreach (RepeaterItem item in repOngoing.Items)
   {
       HtmlInputCheckBox c = (HtmlInputCheckBox)item.FindControl("chkMark");

       if (c.Checked)
       {
           HiddenField hfId = (HiddenField)item.FindControl("hfErrandId");

           ITP1ErrandVO evo = getITP1Errand(new Guid(hfId.Value));
           if (evo.ErrandTypeId == IKConstants.ERRANDTYPE_ITP1_OTHER)
           {
               uncheckErrands();
               lblError.Text = "'Övrigt Ärende' kan ej sättas klar för batch.";
           }
           else
           {
               list = getCheckedErrands();  
           }
       }
   }

The page dislaying a repeater which is feed a list of ITP1ErrandVO:s to produce a list which users can edit and then save. This method is invoked when hitting the "save".

First of all it iterates over all repeateritems finding each checkbox and each id. Then a db-query fetches the errand and populates an object with the rows data. If its type is of type ITP_OTHER then it calls the uncheckErrands method:

private void uncheckErrands()
    {
        foreach (RepeaterItem item in repOngoing.Items)
        {
            HtmlInputCheckBox c = (HtmlInputCheckBox)item.FindControl("chkMark");
            c.Checked = false;
        }
    }

This method also iterate over all items in the repeater finding the checkboxes and unchecking all of them! So in the rest of the iterations the c.Checked is never true. Hence it will call getCheckedErrands() for all else items. Take a look at getCheckedErrands method

private List getCheckedErrands()
    {
        List list = new List();
        foreach (RepeaterItem item in repOngoing.Items)
        {
            HtmlInputCheckBox c = (HtmlInputCheckBox)item.FindControl("chkMark");

            if (c.Checked)
            {
                HiddenField hfId = (HiddenField)item.FindControl("hfErrandId");
                list.Add(new Guid(hfId.Value));
            }
        }
        return list;
    }

So for every unchecked checkbox find all checkboxes that is checked and add them to a list and return.

According to my colleague the errand is very seldom of type ERRANDTYPE_ITP1_OTHER so the list returned from the main-loop will be populated over and over again

Thank god I dont code like this =)