CODEWITHSUNDEEP
c#wpfmultithreadinguser-interfacetask
Furkan Gözükara
Furkan Gözükara

Reputation: 23850

Accessing GUI element is not working as intended

When I run the below code it is supposed fill the ListBox with each DataRow's values, but it is filling the ListBox with single DataRow's value.

What is the problem and how can I solve it? This is a C# 4.0 WPF application.

    Task.Factory.StartNew(() =>
    {
        myThread();
    });

void myThread()
{
    using (DataTable dtTemp = DbConnection.db_Select_DataTable(srQuery))
    {
        foreach (DataRow drw in dtTemp.Rows)
        {   
            this.Dispatcher.BeginInvoke(new Action(delegate()
            {
                listBox1.Items.Add(drw["Value"].ToString());
            }));
        }
    }
}

Upvotes: 1

Views: 115

Answers (2)

Jon Skeet
Jon Skeet

Reputation: 1502206

The problem is that you're capturing the drw variable in your anonymous method. That variable is being updated in the foreach loop, so by the time your delegates are invoked on the dispatcher thread, you're getting the same value each time. In C# 5, this would be okay (it's such a common error that the language has changed to avoid it) but before C# 5 you need to copy the variable in the loop:

foreach (DataRow drw in dtTemp.Rows)
{   
    DataRow copy = drw;
    this.Dispatcher.BeginInvoke(new Action(delegate()
    {
        listBox1.Items.Add(copy["Value"].ToString());
    }));
}

Or even better, do all the data access in the background thread:

foreach (DataRow drw in dtTemp.Rows)
{   
    string item = drw["Value"].ToString();
    this.Dispatcher.BeginInvoke(new Action(delegate()
    {
        listBox1.Items.Add(item);
    }));
}

Note that the change to C# 5 only affects foreach - not for loops.

Also note that your code can be slightly shorted using a lambda expression instead of an anonymous method:

foreach (DataRow drw in dtTemp.Rows)
{   
    string item = drw["Value"].ToString();
    Action action = () => listBox1.Items.Add(item);
    this.Dispatcher.BeginInvoke(action);
}

Upvotes: 6

Kendall Frey
Kendall Frey

Reputation: 44376

Variable capturing strikes again. (Google: variable capture)

Try creating a temporary variable and passing that to the thread, like this:

void myThread()
{
    using (DataTable dtTemp = DbConnection.db_Select_DataTable(srQuery))
    {
        foreach (DataRow drw in dtTemp.Rows)
        {
            DataRow tmp = drw;
            this.Dispatcher.BeginInvoke(new Action(delegate()
            {
                listBox1.Items.Add(tmp["Value"].ToString());
            }));
        }
    }
}

Upvotes: 4

Related Questions