Wednesday, October 7, 2009

Musings on a ComboBox


I just spent a good half an hour debugging a control I thought I had a pretty good handle on, the ComboBox.

In .NET 3.5 we have a really neat control called the ComboBox, it's a bit of a textbox, and a bit of a dropdown list box. However the eventing seems a little wonky.

I had the following code:

private void MyComboBox_SelectedIndexChanged(object sender, EventArgs e)
{
    if (!string.IsNullOrEmpty(TargetBox.Text))
    {
        if (MessageBox.Show("There is text in the request textbox, do you want to replace this?",
            "Replace Text?", MessageBoxButtons.YesNo, MessageBoxIcon.Question) ==
            DialogResult.Yes)
        {
            TargetBox.Text = File.ReadAllText(MyComboBox.SelectedText);
        }
    }
    else
        if (!string.IsNullOrEmpty(MyComboBox.SelectedText))
            TargetBox.Text = File.ReadAllText(MyComboBox.SelectedText);
}

Seems simple enough, right? I'm just grabbing whatever the user just selected out of the dropdown (which happens to be a file in my case) and read that file and populate a textbox with the new value. When there was already text in the text box I'd display up a MessageBox warning the user I'm about to blow away the existing text.

Here's the interesting part, when there was text in the TextBox already (so I displayed the MessageBox) everything worked as expected. However, when there was no text in the TextBox I always got the previous value of the ComboBox, as if the ComboBox hadn't updated its state yet. This occurred if I removed all the MessageBox code and just tried to set read the filetext directly.

The fix? Get the newly SelectedIndex of the ComboBox directly, lookup the value of the item that's selected from the Items list, store that value in a string and reference that throughout.

Final Code:
private void MyComboBox_SelectedIndexChanged(object sender, EventArgs e)
{
    string selectedFile = MyComboBox.Items[MyComboBox.SelectedIndex].ToString();
    if (!string.IsNullOrEmpty(TargetBox.Text))
    {
        if (MessageBox.Show("There is text in the request textbox, do you want to replace this?",
            "Replace Text?", MessageBoxButtons.YesNo, MessageBoxIcon.Question) ==
            DialogResult.Yes)
        {
            TargetBox.Text = File.ReadAllText(selectedFile);
        }
    }
    else
        if (!string.IsNullOrEmpty(selectedFile))
            TargetBox.Text = File.ReadAllText(selectedFile);

If you see something I missed, please let me know in the comments. I am a firm believer in clean code and would love to learn how to make this better.

Labels: , ,


Comments: Post a Comment





<< Home

This page is powered by Blogger. Isn't yours?

Subscribe to Posts [Atom]