Store multiple ListBox selections in database?

Question

I have a ListBox called listbox. Its "multiple selection" property is set to true. I need to store the selections from this ListBox into a database field.

Note that I am using web forms, ASP.NET, C#, and Visual Studio Web Developer 2010 Express.

My code is as follows:

SqlCommand insertNL = new SqlCommand("insert into dbo.newsletter (newsletter_subject,newsletter_body,newsletter_sentto) VALUES ('" + TextBox1.Text + "', '" + TextBox2.Text + "', '" + ListBox1.SelectedItem + "')", badersql);
        badersql.Open();
        insertNL.ExecuteNonQuery();
        badersql.Close();

Unfortunately, this code only stores the first selected value of the ListBox in the "newsletter_sentto" column of my newsletter table. Does anyone have any suggestions as to how to fix this code? Thanks in advance.

did any of these answers solve your question? if so, select that one as "correct". If not, edit your question and/or add comments to better clarify what you are after, so you can get a better answer. — KM., Dec 15 '10 at 20:31

score 3 · Answer 1 · edited May 23 '17 at 09:58

Things to fix:

Before doing anything else, parameterize your sql. This is ripe for injection. Tutorial here.
You aren't disposing of your cmd or connection string. Wrap those in Using clauses. Example here.
Do a foreach on the items to see which ones are selected. Either store those in a comma separated list in the database (which needs parsing on the way back out) or store them in their own table.

score 2 · Answer 2 · answered Dec 09 '10 at 20:13

You need to decide how you want to store the multiple "newsletter_sentto" values first.

You're best solution is to create a new child table where you have 1 row and column per selected item, with a foreign key back to your newsletter table.
You can try to store them all together in one row with multiple columns (sentto1, sentto2, etc), this will limit the max number of values you can store, and will cause problems searching across multiple fields. How will you query what was sent to a particular person? WHERE sentto1=@user or sentto2=@user... no index can be used there.
You can stuff all the value in a single row and column using a "," or ";" to separate the values. this will cause many problems because you'll have to constantly split the string apart, every time you need to get at one of the sentto values.

Thomas · Answer 3 · 2010-12-09T20:27:19.900

First, you should use parametrized queries instead of straight concatenation to protect against SQL injection. Second, you need to cycle through the selected items and build (presumably) a delimited-list of the selections.

var selectedItems = new List<string>();
foreach( var item in ListBox1.SelectedItems )
    selectedItems.Add( item.ToString() );

var sql = "Insert dbo.newsletter(newsletter_subject,newsletter_body,newsletter_sentto)"
    + " Values(@newsletter_subject, @newsletter_body, @newsletter_sentto)"

badersql.Open();
using ( qlCommand = new SqlCommand(sql, badersql) )
{
    qlCommand.Parameters.AddWithValue("@newsletter_subject", TextBox1.Text);
    qlCommand.Parameters.AddWithValue("@newsletter_body", TextBox2.Text);
    qlCommand.Parameters.AddWithValue("@newsletter_sentto", string.Join(',', selectedItems.ToArray()));
    qlCommand.ExecuteNonQuery();
};

score 1 · Answer 4 · answered Dec 09 '10 at 20:11

1

Try to get selected items like that :

string newsletterSentTo = "";
foreach (ListItem item in ListBox1.Items)
{
    if (item.Selected)
        newsletterSentTo += "," + item.Text;
}

answered Dec 09 '10 at 20:11

Canavar

47,715
17
91
122

I think using a StringBuilder and appending to it be more efficient in this case, instead of creating new string for each item. – EndlessSpace Dec 09 '10 at 20:15

Store multiple ListBox selections in database?

4 Answers4