2

I have a simple WPF window with: Loaded="StartTest" and

<Grid>
        <ListBox ItemsSource="{Binding Logging, IsAsync=True}"></ListBox>
</Grid>

In code behind I have in method StartTest:

LogModel LogModel = new LogModel();

void StartTest(object sender, RoutedEventArgs e)
{
    DataContext = LogModel;

    for (int i = 1; i<= 10; i++)
    {
       LogModel.Add("Test");
       Thread.Sleep(100);
    }
}

And class LogModel is:

public class LogModel : INotifyPropertyChanged
{
    public LogModel()
    {
        Dispatcher = Dispatcher.CurrentDispatcher;
        Logging = new ObservableCollection<string>();
    }
    Dispatcher Dispatcher;

    public ObservableCollection<string> Logging { get; set; }

    public event PropertyChangedEventHandler PropertyChanged;

    public void Add(string text)
    {
        Dispatcher.BeginInvoke((Action)delegate ()
        {
            Logging.Add(text);
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs("Logging"));
        });
    }
}

Of course the problem is that the UI doesn't update in the loop. What am I missing?
How can I achieve the UI update?

Tatranskymedved
  • 4,194
  • 3
  • 21
  • 47
Gerard
  • 13,023
  • 14
  • 72
  • 125
  • Setting `IsAsync=True` is useless in this case. – dymanoid May 25 '18 at 07:57
  • Yes, I added it to avoid it as a suggested solution. – Gerard May 25 '18 at 07:58
  • I meant to post this answer actually. Sorry: https://stackoverflow.com/a/20148603/7167572 – Tatranskymedved May 25 '18 at 08:02
  • 1
    The dispatcher was used *before* the introduction of `async/await` and async data binding. You *don't* need to raise the event on the UI thread anyway, it's the *View's* job to update itself from the data. Since you use an `ObservableCollection` you didn't need to raise the event at all, it's raised by the collection itself. `StartTest` blocks the UI thread itself. Too many weird things anyway – Panagiotis Kanavos May 25 '18 at 08:03
  • 1
    @Gerard looks like you hit a snag at some point and started throwing code to "fix" it? What was your original intention? Update the UI periodically with log messages? You could use a simple model with just one `Logging` property. As long as `StartTest` doesn't block, ie no `Thread.Sleep` you shouldn't have any issues. Use `await Task.Delay()` instead. BTW, are you setting the view's context to the model? – Panagiotis Kanavos May 25 '18 at 08:04
  • @PanagiotisKanavos I agree I miss an essential point. Indeed I want to only add a log message after blocking UI thread 100ms. So the block is intentional. – Gerard May 25 '18 at 08:09
  • @Gerard then you *can't* update the UI if it's already blocked. If you want to update the UI every 100 ms use `Task.Delay()` – Panagiotis Kanavos May 25 '18 at 08:11
  • @PanagiotisKanavos I wish I could understand that. I know of `Task.Delay()` but *after* `Thread.Sleep` the UI is no longer blocked. The code behind is the view and responsible for updating UI. At that point I want an UI update? – Gerard May 25 '18 at 08:27
  • @Gerard the UI thread is still blocked because the *loop* still runs and calls `Add;Sleep` over and over. The UI thread is never allowed to process other events – Panagiotis Kanavos May 25 '18 at 08:36

3 Answers3

2

ObservableCollection already raises the PropertyChanged event when it's modified. You don't have to raise the event in the UI thread either.

Your model can be as simple as :

class LogModel
{
    public ObservableCollection<string> Logging { get; } = new ObservableCollection<string>();

    public void Add(string text)
    {
        Logging.Add(text);
    }
}

All you need to do is set it as the DataContext of your view, eg :

LogModel model = new LogModel();
public MainWindow()
{
    InitializeComponent();
    this.DataContext = model;
}

I assume StartTest is a click handler which means it runs on the UI thread. That means it will block the UI thread until the loop finishes. Once the loop finishes the UI will be updated.

If you want the UI to remain responsive during the loop, use Task.Delay instead of Thread.Slepp, eg :

private async void Button_Click(object sender, RoutedEventArgs e)
{
    for(int i=0;i<10;i++)
    {
        await Task.Delay(100);
        model.Add("Blah!");
    }
}

Update

You don't need to use an ObservableCollection as a data binding source. You could use any object, including an array or List. In this case though you'd have to raise the PropertyChanged event in code :

class LogModel:INotifyPropertyChanged
{
    public List<string> Logging { get; } = new List<string>();

    public event PropertyChangedEventHandler PropertyChanged;

    public void Add(string text)
    {
        Logging.Add(text);
        PropertyChanged.Invoke(this,new PropertyChangedEventArgs("Logging"));
    }
}

This will tell the view to load all the contents and display them again. This is perfectly fine when you only want to display data loaded eg from the database without modifying them, as it makes mapping entities to ViewModels a lot easier. In this case you only need to update the view when a new ViewModel is attached as a result of a command.

This is not efficient when you need to update the coolection though. ObservableCollection implements the INotifyCollectionChanged interface that raises an event for each change. If you add a new item, only that item will be rendered.

On the other hand you should avoid modifying the collection in tight loops because it will raise multiple events. If you load 50 new items, don't call Add 50 times in a loop. Create a new ObservableCollection, replace the old one and raise the PropertyChanged event, eg :

class LogModel:INotifyPropertyChanged
{
    public ObservableCollection<string> Logging { get; set; } = new ObservableCollection<string>();

    public event PropertyChangedEventHandler PropertyChanged;

    public void Add(string text)
    {            
        Logging.Add(text);
        PropertyChanged.Invoke(this,new PropertyChangedEventArgs("Logging"));
    }

    public void BulkLoad(string[] texts)
    {
        Logging = new ObservableCollection<string>(texts);
        PropertyChanged.Invoke(this, new PropertyChangedEventArgs("Logging"));
    }
}

The explicit implementation is still needed because the Logging property is getting replaced and can't raise any events itself

Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
1

The reason why the UI is not updated in the loop is a call to Dispatcher.BeginInvoke. This places a new DispatcherOperation in the dispatcher queue. But your loop is already a dispatcher operation, and it continues on the Dispatcher's thread. So all the operations you queue will be executed after the loop's operation is finished.

Maybe you wanted to run the StartTest on a background thread? Then, the UI will update.

By the way, don't block the Dispatcher's thread with Thread.Sleep. It prevents the Dispatcher from doing its things as smoothly as possible.

dymanoid
  • 14,771
  • 4
  • 36
  • 64
0

It is the DoEvents thing, overhere:

public static void DoEvents()
{
    Application.Current.Dispatcher.Invoke(DispatcherPriority.Background,
                                          new Action(delegate { }));
}

or even the perhaps better https://stackoverflow.com/a/11899439/138078.

Of course the test should be written differently in a way which does not require it.

Gerard
  • 13,023
  • 14
  • 72
  • 125