checkboxlist selection issue

There are 5 goals and 5 employees. Each goal can be assigned to any number of these 5 employees. So I have 5 CheckBoxLists for each of the goals, each CheckBoxList having the names of these 5 employees as items.

I want to retrieve from the database which employees have been assigned which goals. I have the following piece of code:

            List<CheckBoxList> checkboxlists = new List<CheckBoxList>();
            checkboxlists.Add(CheckBoxList1);
            checkboxlists.Add(CheckBoxList2);
            checkboxlists.Add(CheckBoxList3);
            checkboxlists.Add(CheckBoxList4);
            checkboxlists.Add(CheckBoxList5);


            for (int z = 1; z <= checkboxlists.Count; z++)
            {
                SqlCommand check = new SqlCommand("SELECT ISGoal1, ISGoal2,ISGoal3, ISGoal4,ISGoal5 FROM PRM2011_EMPLOYEE_GOAL WHERE EmployeeID = '" + employeeid[z - 1] + "'", con);
                SqlDataReader y = check.ExecuteReader();

                y.Read();

          开发者_如何学C      for (int j = 1; j <= 5; j++)
                {
                    if (null != y && y.HasRows)
                    {
                        string yes_or_no = y["ISGoal" + j].ToString().Trim();
                        if (yes_or_no == "Yes")
                        {
                            checkboxlists[j-1].Items[z-1].Selected = true;

                        }
                        //else checkboxlists[j - 1].Items[z - 1].Selected = false;
                    }
                }

                y.Close();
           }

My problem is that even if I select one goal for an employee, all the checkboxes corresponding to that particular employee get selected. Why is this happening?

Correspondingly, if I comment out the else portion in the code posted, and if any of the goals are not selected, then all the checkboxes corresponding to that employee goes unselected. Please help.

A few thoughts:

1. Have your for loops go from 0 to less than checkboxlists.Count and from 0 to less than 5. That way you can avoid having to deal with all the subtractions everywhere.
2. On the line checkboxlists[j-1].Items[z-1].Selected = true, shouldn't it be checkboxlists[z-1].Items[j-1].Selected = true since I am assuming you are using z to iterate over your CheckBoxLists.

Its late here, so my brain may be a bit fuzzy, but it seems that #2 could be your issue. Give those thoughts a shot and I will follow up with you if you are still having issues.

You have many issues in your code I'll tell you the main problem and then I'll list the other flaws of your code.

What's causing this is:

the existence of the method y.Read(); out side of the for loop. since the function of Read(); is to read the next row in the database. so basically your code reads the first value, Let's assume the value will be "Yes" so it will cause the ListBox to check the employee and instead of calling the y.Read(); again so it moves to the next row it DOES NOT! .. so the value is kept "Yes" and hense all the CheckBoxes in the List will be checked.

The Solution:

It's as simple as just moving the y.Read(); from out side the loop into it.

Like this:

        List<CheckBoxList> checkboxlists = new List<CheckBoxList>();
        checkboxlists.Add(CheckBoxList1);
        checkboxlists.Add(CheckBoxList2);
        checkboxlists.Add(CheckBoxList3);
        checkboxlists.Add(CheckBoxList4);
        checkboxlists.Add(CheckBoxList5);


        for (int z = 1; z <= checkboxlists.Count; z++)
        {
            SqlCommand check = new SqlCommand("SELECT ISGoal1, ISGoal2,ISGoal3, ISGoal4,ISGoal5 FROM PRM2011_EMPLOYEE_GOAL WHERE EmployeeID = '" + employeeid[z - 1] + "'", con);
            SqlDataReader y = check.ExecuteReader();


            for (int j = 1; j <= 5; j++)
            {
               y.Read();

                if (null != y && y.HasRows)
                {
                    string yes_or_no = y["ISGoal" + j].ToString().Trim();
                    if (yes_or_no == "Yes")
                    {
                        checkboxlists[j-1].Items[z-1].Selected = true;

                    }
                    //else checkboxlists[j - 1].Items[z - 1].Selected = false;
                }
            }

            y.Close();
       }

Extra Notes on your code

First of all

You need to edit your SqlCommand to use SqlParameters

SqlCommand check = new SqlCommand("SELECT ISGoal1, ISGoal2,ISGoal3, ISGoal4,ISGoal5 FROM PRM2011_EMPLOYEE_GOAL WHERE EmployeeID = @EmpID", con);

check.Parameters.AddWithValue("@ImpID", employeeid[z - 1]);

Second

If you are trying to build a real application then this is a very bad practice. Even if you are not building this application for real I don't think this is the way to practice.

for (int j = 1; j <= 5; j++)

your Loop should look like this:

for (int j = 1; j <= checkboxlists.Count; j++)

Third

Using a string to represent a Yes/No value is a also a bad practice .. You should use for all your ISGoal columns database, the DataType BIT. and consequently you'll change the local variable's DateType in your C# code from string to bool.

Fourth

checkboxlists[j-1].Items[z-1].Selected = true;

You should switch as Ryan said, because z denotes to the CheckBoxLists and j denotes to the items of a given CheckBoxList

so it could be like this:

checkboxlists[z-1].Items[j-1].Selected = true;

N.B: I wasn't paying attention at first I thougth [z-1] is some kind of LINQ expression :D!! .. It's my fault but I mean I used to do this when I first started programming and I still couldn't recognize it .. It's not the best practice I think! .. my advice respect the used Zero-Based numbering.

Finally

You don't have to check everytime if y.Read(); is null or not. So I think this piece of code would make more sense and also hard coding the condition of the loop is a very bad practice, so we'll use a while loop and add local variable of type int, then we'll increment it within our loop code so you could use it to access the CheckBoxList items

                int j = 1;
                while (y.Read())
                {
                        string yes_or_no = y["ISGoal" + j].ToString().Trim();
                        if (yes_or_no == "Yes")
                        {
                            checkboxlists[j-1].Items[z-1].Selected = true;
                            //use our counter "j" here

                        }
                        //else checkboxlists[j - 1].Items[z - 1].Selected = false;
                        //use our counter "j" here
                        j++;
                    }

..Good Luck ;)