1
\$\begingroup\$

I got a database with members, each member has a list of sports they do. now I want to loop through a listbox and add every selected item to my database.

This is my database :

enter image description here

And this is my code :

foreach (var item in sportCheckedListBox.CheckedIndices)
            {
                int sportindex = Convert.ToInt32(item.ToString()) + 1;
                var queryResult = from sp in context.Sports
                                  where sp.sportnr == sportindex
                                  select sp;
                foreach (var sport in queryResult)
                {
                    myMember.Sports.Add(sport);
                }
            }

This looks kinda 'shady', how could I do this better ?

\$\endgroup\$
1
  • \$\begingroup\$ I'd create a list of ints from sportCheckedListBox.CheckedIndices, then use that list to query your Sports Table. Once you have your result I believe you can just do AddRange on myMember.Sports instead of doing a foreach over the result. This should prevent you from having to use foreach at all \$\endgroup\$
    – wentimo
    Jan 7, 2016 at 19:02

1 Answer 1

2
\$\begingroup\$

You're doing a lot of unnecessary iterating with your nested for loops. I would suggest using CONTAINS over a list of ints for your SQL query, this will eliminate your outer loop. Your inner loop is adding each individual item whereas you can use AddRange to just add them all.

//Generate a List of Integers from your Indices:
int[] indexes = myCheckedListBox.CheckedIndices.Cast<int>().ToArray()

//Use that list of Integers to get your list of Sports Objects
var queryResult = from sp in context.Sports 
                  where indexes.Contains(sp.sportnr)

//Add your results. 
myMember.Sports.AddRange(queryResult);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.