-1

Iam currently working on a book management project and I'm using SQL Server from Visual Studio. I have a Book Category table in the database and I'm trying to place it in a combobox.

This the code - I don't see anything wrong with it but the categories are taking so long to be visible in the combobox.

Also the list is repetitive, is it maybe because of the while Loop? If so is there any way to fix it?

private void comboBox1_SelectedIndexChanged(object sender, EventArgs e)
        {
            try
            {
                con.ConnectionString = (@"Data Source=(LocalDB)\MSSQLLocalDB;AttachDbFilename=C:\Users\malek\source\repos\BookStore\BookStore\BOOKDB.mdf;Integrated Security=True");
                scmd.Connection = con;
                con.Open();
                scmd.CommandText = "SELECT CATEGORY FROM BOOKCAT";
                var rd = scmd.ExecuteReader();
                while (rd.Read())
                {
                    List.Add(Convert.ToString(rd[0]));
                }
                int i = 0;
                while (i < List.LongCount())
                {
                    comboBox1.Items.Add(List[i]);
                    i = i + 1;
                }

            }
            catch (Exception EX)
            {

                MessageBox.Show(EX.Message);
            }
            finally
            {
                con.Close();
            }
        }

What did I miss?

NOTE: I am not getting any errors!!

  • What kind of combo box? Winforms? WPF? Webforms? Maui? Xamarin? Other? – Robert Harvey Feb 11 '22 at 22:26
  • In any case, this can almost certainly be done in a much simpler way, generally with a Data Binding. – Robert Harvey Feb 11 '22 at 22:27
  • Well its Winforms. How do you mean with Data Binding? –  Feb 11 '22 at 22:42
  • Side note: Your `while` loop is a strange choice. A `for` loop would be much more idiomatic for what you're doing. – madreflection Feb 11 '22 at 22:47
  • 1
    `LongCount` iterates through the collection every time. It can't be optimized because the optimizations used by `Count()` rely on `int` properties. That's what's bogging it down. You have an `O(n^2)` operation. Do you really think you might have more than 2.4 *billion* items in this list? And since you're declaring your counter `i` as `int`, it'll overflow if you do and end up being an infinite loop. – madreflection Feb 11 '22 at 22:49
  • [How to bind a list to a combo box in Winforms](https://stackoverflow.com/q/600869/102937) – Robert Harvey Feb 11 '22 at 23:42

2 Answers2

1

How do you mean with data binding?

Like this

var da = new SqlDataAdapter(
  "SELECT DISTINCT CATEGORY FROM BOOKCAT ORDER BY CATEGORY"
  @"Data Source=(LocalDB)\MSSQLLocalDB;AttachDbFilename=C:\Users\malek\source\repos\BookStore\BookStore\BOOKDB.mdf;Integrated Security=True"
);
var dt = new DataTable();
da.Fill(dt);
categoryComboBox.DisplayMember = "CATEGORY";
categoryComboBox.ValueMember = "CATEGORY";
categoryComboBox.DataSource = dt;

And when you want the thing the user selected:

 var cat = categoryComboBox.SelectedValue as string;

Simple eh?

It gets even easier if you use a strongly typed dataset; for that you just add a new DataSet type file to your project (must use net framework not net core/5+), drag your db into your dataset, add a query to the category TableAdapter that gets the distinct categories (like above) then open the data sources window, change Category to a combo and tag it onto the form. No code to write; vs will write it all

Caius Jard
  • 72,509
  • 5
  • 49
  • 80
0

Based on the code you've posted, it looks like you're loading the categories from the database in the SelectedIndexChanged event of comboBox1.

So, every time you choose a new item from comboBox1 you're executing this code; you're going to the database and loading everything from the BOOKCAT table and you're putting those items into List and comboBox1. That is why you're seeing duplicated categories. It's probably also why it takes so long for a category to be visible in the ComboBox.

You probably don't want to load the ComboBox items from the database every time the selected index changes, so you should do that somewhere else. For example, you could do it in the Form's constructor, or in the 'Load' event.

Joshua Robinson
  • 3,399
  • 7
  • 22