CODEWITHSUNDEEP
c#foreachcombobox
Sean
Sean

Reputation: 442

Iterating through USB Drives

I am iterating through all connected USB drives on the Form_Load event and populating them into a Combobox. This works fine, however, I now have a button that I want to use to refresh the list and add any to the list, that do not exist already.

This is the code I'm using: 

private void btnRefresh_Click(object sender, EventArgs e)
{
    try
    {
        DriveInfo[] allDrives = DriveInfo.GetDrives();

        foreach (DriveInfo d in allDrives)
        {
            if (d.IsReady == true)
            {   
                string dl = d.VolumeLabel;
                string dt = Convert.ToString(d.DriveType); 

                if (comboBox1.Items.Contains(comboBox1.Text))
                {

                }  
                else
                {  
                    comboBox1.Items.Add(d.Name.Remove(2));
                }
            }
            comboBox1.SelectedIndex = 0;
            comboBox1.DropDownStyle = ComboBoxStyle.DropDownList;
        }
    }
    catch { MessageBox.Show("Error retrieving Drive Information", "Error!"); }
}

I'm not sure where I've gone wrong and I need another pair of eyes, but when I have the form open and hit the button, the newly added drives do not populate into my Combobox.

Any help would be appreciated.

Upvotes: 0

Views: 190

Answers (3)

Thewads
Thewads

Reputation: 5063

In your 2nd If you should be checking for d.Name, and not the comboBox1.Text itself. You are also not clearing the list after initial population from Form_Load

I would suggest pulling out the population code into its own method and calling from both Form_Load and btnRefresh_Click rather than duplicating the logic.

The new method would look something like this:

private void PopulateDrives()
{
    try
    {
        combobox.items.clear()
        DriveInfo[] allDrives = DriveInfo.GetDrives();

        foreach (DriveInfo d in allDrives)
        {
            if (d.IsReady == true)
            {   
                string dl = d.VolumeLabel;
                string dt = Convert.ToString(d.DriveType); 

                comboBox1.Items.Add(d.Name.Remove(2));
            }
            comboBox1.SelectedIndex = 0;
            comboBox1.DropDownStyle = ComboBoxStyle.DropDownList;
        }
    }
    catch { MessageBox.Show("Error retrieving Drive Information", "Error!"); }
}

You would then simply call PopulateDrives() from both places. I have also just inverted your 2nd if statement to tidy it up a little

Upvotes: 1

online Thomas
online Thomas

Reputation: 9391

I'd suggest:

private void btnRefresh_Click(object sender, EventArgs e)
{
    try
    {
        comboBox1.Items.Clear();
        DriveInfo[] allDrives = DriveInfo.GetDrives();

        foreach (DriveInfo d in allDrives)
        {
            if (d.IsReady)
            {   
                string dl = d.VolumeLabel;
                string dt = Convert.ToString(d.DriveType); 
                comboBox1.Items.Add(d.Name.Remove(2));
            }
        }
            comboBox1.SelectedIndex = 0;
            comboBox1.DropDownStyle = ComboBoxStyle.DropDownList;
    }
    catch { MessageBox.Show("Error retrieving Drive Information", "Error!"); }
}

Upvotes: 1

TheLethalCoder
TheLethalCoder

Reputation: 6744

This line:

if (comboBox1.Items.Contains(comboBox1.Text))

is checking if comboBox1 contains comboBox1's title text, which it does.

You should be checking if it contains either dl or dt or d.Name

Upvotes: 2

Related Questions