-1

I have a TextBox with a TextChanged event that calls a custom event if the text of the textbox represents an existing file. In this event, there is a call to an outside dll that does some processing on the File, which can take upwards of a minute to finish. There is also some post-processing I do, dependent on what result this method returns to me. Currently, this is blocking my UI, which is highly undesirable.

There are essentially 2 "options"/scenarios I see.

  1. Within the custom event, somehow wait for the dll call to finish, before continuing the event, while also keeping the UI free. This seems like the simplest idea from my multithreading-untrained self, but it also conceptually throws red flags at me: Is this even possible given that the custom event itself (called from TextChanged) is on the UI thread?
  2. Throw the entire custom event into it's own thread using Task.Run(). Downside here is that apart from the dll method call, there is quite a good amount of UI elements that are affected by getters/setters after the long method. I could write alternated getters/setters based on the appropriate InvokeRequired, but if there is a more correct way to do this, I'd rather take that approach.

I made a much shorter (although contrived) example project, which shows essentially what I'm after, using option 2 from above:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();

        comboBox1.Items.Add("Select One...");
        comboBox1.Items.Add("Item 1");
        comboBox1.Items.Add("Item 2");

        Value = 0;
    }

    public string SetMessage
    {
        set
        {
            if (lblInfo.InvokeRequired)
                lblInfo.BeginInvoke((MethodInvoker)delegate () { lblInfo.Text = Important ? value + "!" : value; });
            else
                lblInfo.Text = Important ? value + "!" : value;
        }
    }

    public bool Important
    {
        get
        {
            return chkImportant.Checked;
        }
        set
        {
            if (chkImportant.InvokeRequired)
                chkImportant.BeginInvoke((MethodInvoker) delegate() { chkImportant.Checked = value; });
            else
                chkImportant.Checked = value;
        }
    }

    public SomeValue Value
    {
        get
        {
            if (comboBox1.InvokeRequired)
            {
                SomeValue v = (SomeValue)comboBox1.Invoke(new Func<SomeValue>(() => SomeValue.Bar));
                return v;
            }
            else
            {
                switch (comboBox1.SelectedIndex)
                {
                    case 1:
                        return SomeValue.Foo;
                    case 2:
                        return SomeValue.Bar;
                    default:
                        return SomeValue.Nothing;
                }
            }
        }
        set
        {
            if (comboBox1.InvokeRequired)
            {
                comboBox1.BeginInvoke((MethodInvoker)delegate ()
                {
                   switch (value)
                   {
                       case SomeValue.Nothing:
                           comboBox1.SelectedIndex = 0;
                           break;
                       case SomeValue.Foo:
                           comboBox1.SelectedIndex = 1;
                           break;
                       case SomeValue.Bar:
                           comboBox1.SelectedIndex = 2;
                           break;
                   }
                });
            }
            else
            {
                switch (value)
                {
                    case SomeValue.Nothing:
                        comboBox1.SelectedIndex = 0;
                        break;
                    case SomeValue.Foo:
                        comboBox1.SelectedIndex = 1;
                        break;
                    case SomeValue.Bar:
                        comboBox1.SelectedIndex = 2;
                        break;
                }
            }
        }
    }

    private void CustomEvent(object sender, EventArgs e)
    {
        if (!Important)
            Important = true;

        SetMessage = "Doing some stuff";

        if (Value == SomeValue.Foo)
            Debug.WriteLine("Foo selected");

        //I don't want to continue until a result is returned, 
        //but I don't want to block UI either.
        if (ReturnsTrueEventually())
        {

            Debug.WriteLine("True!");
        }

        Important = false;
        SetMessage = "Finished.";
    }

    public bool ReturnsTrueEventually()
    {
        //Simulates some long running method call in a dll.
        //In reality, I would interpret an integer and return 
        //an appropriate T/F value based on it.
        Thread.Sleep(5000);

        return true;
    }

    private void textBox1_TextChanged(object sender, EventArgs e)
    {
        //Do I *need* to multithread the whole thing?
        Task.Run(() => CustomEvent(this, new EventArgs()));
    }
}

public enum SomeValue
{
    Nothing = 0,
    Foo = 100,
    Bar = 200
}

Note: I'm not asking for code review on my option 2 code. Rather, I'm asking if option 2 is necessary to accomplish, since that option causes me to change a considerably larger portion of code, given that it's only 1 method within it holding up the entire process.

I also realize I can simplify some of the code in these properties to prevent replication. For the sake of demonstrating to myself and debugging, I am holding off on that at this time.

Here is what I had related to option 1 (left out duplicate code and the getters/setters without their invokes):

private async void CustomEvent(object sender, EventArgs e)
{
    if (!Important)
        Important = true;

    SetMessage = "Doing some stuff";

    if (Value == SomeValue.Foo)
        Debug.WriteLine("Foo selected");

    //I don't want to continue until a result is returned, 
    //but I don't want to block UI either.
    if (await ReturnsTrueEventually())
    {

        Debug.WriteLine("True!");
    }

    Important = false;
    SetMessage = "Finished.";
}

public async Task<bool> ReturnsTrueEventually()
{
    //Simulates some long running method call in a dll.
    //In reality, I would interpret an integer and 
    //return an appropriate T/F value based on it.
    Thread.Sleep(5000);

    return true;
}
Broots Waymb
  • 4,713
  • 3
  • 28
  • 51
  • If you want to compare the two options, write up option 1, so you can see how it's different. – Servy Aug 04 '17 at 19:23
  • Use async/await and tasks to keep your UI responsive. What version of the .NET framework are you using? – Dido Aug 04 '17 at 19:24
  • @Servy - When I did that, I basically came up with some that ending in me checking `await ReturnsTrueEventually().Result` which if I understand correctly, ends up blocking anyway. Of course, I realize I could be taking the wrong approach entirely... – Broots Waymb Aug 04 '17 at 19:41
  • @zzxyz - Tried with async/await. Kept finding myself needing to check .Result (see previous comment). .NET version of the app is 4.5. The library is 4.0 (I don't have control over upgrading that) – Broots Waymb Aug 04 '17 at 19:42
  • 1
    @DangerZone You're correct that you shouldn't ever be calling `Result`. You use `await` to get the results of a `Task`, not `Result`. Using both simply won't compile. – Servy Aug 04 '17 at 19:44
  • @Servy - (somewhat of a miscommunication on my part, I did try both ways) But since it's waiting within the custom event which is running on the UI thread, won't it block it? This is the behavior I'm trying to avoid. I'll post some option 1 code in a minute. – Broots Waymb Aug 04 '17 at 19:49
  • @DangerZone Try it and find out for yourself, or read a tutorial on the topic (or both). – Servy Aug 04 '17 at 19:51
  • @DangerZone -- if you use await from a thread with a dispatcher (like the UI thread), it will actually return control to the message pump (meaning your UI is responsive). The rest of your function will effectively be "called" once the await returns...more or less :). I deleted my earlier comment because my specific advice was incorrect. Definitely do check out a tutorial. Your *exact* scenario is pretty much what MS did this for, so it shouldn't be hard to find something helpful. – zzxyz Aug 04 '17 at 19:52
  • @Servy - Been reading a few/trying multiple things. That's what leads me to believe I a) either can't do option 1, which from these comments actually sounds like I can (and should) which brings me to: b) that I'm not quite researching the correct thing for my case. – Broots Waymb Aug 04 '17 at 19:57
  • The problem with your attempt an "option 1" is simply that you did it wrong. Marking a method `async` doesn't in and of itself give you asynchronous behavior. It just _allows_ async behavior. It's still up to you to make it async. Given the code you posted, you should be able to `if (await Task.Run(() => ReturnsTrueEventually()))` where `public bool ReturnsTrueEventually()`. Then you can get rid of the `Invoke(...)` code. Note that as far as the `Invoke()` code goes, you can skip the `InvokeRequired` check and just always invoke. That would simplify that (not needed) code somewhat. – Peter Duniho Aug 04 '17 at 23:38
  • See marked duplicates. Get rid of all the `Invoke(...)` stuff (i.e. just access the UI elements directly), use `Task.Run()` to execute _just the long-running part_ (i.e. `ReturnsTrueEventually()`) and `await` the task you run. – Peter Duniho Aug 04 '17 at 23:45

1 Answers1

1

This is basically what you want. I'm violating a couple best-practices here, but just showing it's not that complicated. One thing to keep in mind is that the user can now click this button multiple times in a row. You might consider disabling it before processing. Or you can do a Monitor.TryEnter() to make sure it's not already running.

    private async void buttonProcess_Click(object sender, RoutedEventArgs e)
    {
         textBlockStatus.Text = "Processing...";
         bool processed = await Task.Run(() => SlowRunningTask());
    }

    private bool SlowRunningTask()
    {
        Thread.Sleep(5000);
        return true;
    }
zzxyz
  • 2,953
  • 1
  • 16
  • 31
  • Jesus... I'm pretty sure I tried this but forgot the await before Task.Run (or placed it elsewhere, I cannot remember). I'm facepalming pretty hard right about now... – Broots Waymb Aug 04 '17 at 20:04
  • Keep in mind that what you are awaiting is not Task.Run(), but it's return value, which in this case is Task. So you could do `Task processedTask = Task.Run(...);` and then later when you need the results: `bool processed = await processedTask;` – zzxyz Aug 04 '17 at 21:58