2

First of all, I want to mention that I am kinda new to WPF, I just start learning it a few months ago and I am using code behind atm, going to MVVM later... So, Im trying to create a simple program for my discord bot but I want to keep it responsive whenever a database action is going on, meaning my program cannot block while i am -for example- logging into my program. I have a way to archive this but it feels like i am doing something wrong or using Tasking wrong.

I use await Task.Run(() => { code here }) but then inside this task I also need to use the Dispatcher every time I want to update a control and there is where I wonder if i do this all the correct way, is it normal to use the dispatcher inside a Task.Run every time you need to update controls or am I overusing this / doing it wrong?

Currently it works as intented, but I am worried I am overdoing something or going into the wrong direction because for me it seems silly to use the dispatcher every time for updating a control but maybe this is just the way it has to be, i am still new to WPF, this is also my first program i ever did in WPF

Sorry if my english is not so good, i try my best to explain this

I have a sort of solution but the use of Dispatcher inside Tasking seems to me not the correct way, this is the first time im doing this in WPF, I googled around a lot but see people using background workers or threading instead but this still seems to block my UI

// This button I use to -for this example- login in my program
// I load a ContentPresenter over my program showing its 'loading'
// Then I try login


private async void BtnLogin_Click(object sender, RoutedEventArgs e)
{
  cpLoader.Visibility = Visibility.Visible;
  await TryLoginAsync();
}


// Then here i do the login with tasking
// But here i also need to use dispatcher when updating controls
// So I wonder if this is done correctly
private async Task TryLoginAsync()
{
  var username = txtUsername.Text.Trim();
  var password = txtPassword.Password.Trim();

  if (!string.IsNullOrWhiteSpace(username) && !string.IsNullOrWhiteSpace(password))
  {
    await Task.Run(() =>
    {
      try
      {
        var programAdmin = _dbAdmins.FindBy(x => x.Username.ToLower() == username.ToLower());
        if (programAdmin != null)
        {
          if (string.Equals(EncryptionManager.Decrypt(BotDBConfiguration.EncryptionKey, programAdmin.Password), password))
          {
            programAdmin.DateLastLogin = DateTime.Now;
            programAdmin.TotalLogins += 1;

            _dbAdmins.Update(programAdmin);

            Dispatcher.BeginInvoke(new Action(() =>
            {
              var firstTimeLoginPanel = new UpdatePasswordWindow(programAdmin);
              firstTimeLoginPanel.Show();

              Close();
            }));
          }
          else
          {
            Dispatcher.BeginInvoke(new Action(() =>
            {
              cpLoader.Visibility = Visibility.Collapsed;
              AlertManager.ShowAlertMessage("Password error", "The given password is incorrect, please try again...");
            }));
          }
        }
        else
        {
          Dispatcher.BeginInvoke(new Action(() =>
          {
            cpLoader.Visibility = Visibility.Collapsed;
            AlertManager.ShowAlertMessage("Admin not found", "The given username cannot be found in the admin database, please try again...");
          }));
        }
      }
      catch (Exception)
      {
        Dispatcher.BeginInvoke(new Action(() =>
        {
          cpLoader.Visibility = Visibility.Collapsed;
          AlertManager.ShowAlertMessage("Connection error", "Cannot connect to the database, please try again later...");
        }));
      }
    });
  }
  else
  {
    cpLoader.Visibility = Visibility.Collapsed;
    AlertManager.ShowAlertMessage("Missing information", "Please fill in all the required information...");
  }
  }
Skeiln
  • 23
  • 7
  • 1
    Possible duplicate of [How to run and interact with an async Task from a WPF gui](https://stackoverflow.com/questions/27089263/how-to-run-and-interact-with-an-async-task-from-a-wpf-gui). Also please [see](https://stackoverflow.com/a/27089652/1797425) this answer, great information and very detailed. – Trevor Oct 18 '19 at 14:46
  • 1
    The biggest problem I see here is that you are `Task.Run`ing too big a chunk. See my answer for more – BradleyDotNET Oct 18 '19 at 15:02
  • TBH, anything UI related could be ripped out of this, create a task to return what you need and then handle the UI, throwing everything in there is unrelated to the UI if you are just going after data. IMHO using a `Task` this way is not appropriate; you're off loading work to another thread that doesn't need to be. @BradleyDotNET explains this in his answer. – Trevor Oct 18 '19 at 15:11

2 Answers2

0

There is one simple rule in WPF when it comes to UI controls:

All changes must happen on the UI thread.

By running a background worker, thread, or Task you are getting off the UI thread (which is the whole point, so you can avoid blocking it and making your program unresponsive). So if you need to run UI logic inside a task then marshalling back to the UI thread is necessary. This is what the Dispatcher does.

That said, you almost never have to do this in practice (at least when using Task) because of how async/await work. When you use await it automatically returns control to the calling thread after the Task completes, so if you structured your code as:

// Do some UI stuff
await dbTask;
// Do more UI stuff

instead of one giant task, you would find that your need for Dispatcher goes down dramatically.

As noted in the comments, this post provides a lot of super useful general information on dealing with async/await and tasks: How to run and interact with an async Task from a WPF gui

BradleyDotNET
  • 60,462
  • 10
  • 96
  • 117
  • All of this is explained in great detail already as you mentioned as well as I, why iterate it, hence why I haven't answered. TBH, we don't need another duplicate post, under `Related` and `Linked` shows we have a great deal of answer's already on this. – Trevor Oct 18 '19 at 15:01
  • @Çöđěxěŕ That post is extremely generic, and does not directly offer an answer to the question "Why am I using dispatcher so much?" It doesn't even talk about the WPF GUI until 3/4 of the way down. Its a great reference, but I feel that this question can have a direct answer of its own. As far as linked and related, that's the only linked post and I don't see *any* useful info in Related. – BradleyDotNET Oct 18 '19 at 15:03
  • Both thanks for your comments, i am currently reading this other post – Skeiln Oct 18 '19 at 15:07
0

I think you have the right idea, but I agree that your implementation could use a bit of work.

In your sample above, it seems like you're doing one "background" activity (validating the username and password) and then either continuing with the program if it succeeds, or displaying one of several error messages. To make things cleaner, reduce code duplication and minimize the number of places where you jump between threads, I would just use a string variable to store the error message you want to show, and then at the end, tell the main thread the result of the background activity and have it either continue or display the error:

        private async Task TryLoginAsync()
        {
            var username = txtUsername.Text.Trim();
            var password = txtPassword.Password.Trim();

            string errorTitle;
            string errorDetail;
            /*Your Type*/ object programAdmin;
            bool result;

            if (!string.IsNullOrWhiteSpace(username) && !string.IsNullOrWhiteSpace(password))
            {
                result = await Task.Run(new Func<bool>(() =>
                {
                    try
                    {
                        programAdmin = _dbAdmins.FindBy(x => x.Username.ToLower() == username.ToLower());
                        if (programAdmin != null)
                        {
                            if (string.Equals(EncryptionManager.Decrypt(BotDBConfiguration.EncryptionKey, programAdmin.Password), password))
                            {
                                programAdmin.DateLastLogin = DateTime.Now;
                                programAdmin.TotalLogins += 1;

                                _dbAdmins.Update(programAdmin);

                                return true;
                            }
                            else
                            {
                                errorTitle = "Password error";
                                errorDetail = "The given password is incorrect, please try again...";
                            }
                        }
                        else
                        {
                            errorTitle = "Admin not found";
                            errorDetail = "The given username cannot be found in the admin database, please try again...";
                        }
                    }
                    catch (Exception)
                    {
                        errorTitle = "Connection error";
                        errorDetail = "Cannot connect to the database, please try again later...";
                    }

                    return false;
                }));
            }
            else
            {
                errorTitle = "Missing information";
                errorDetail = "Please fill in all the required information...";
            }

            if (result)
            {
                var firstTimeLoginPanel = new UpdatePasswordWindow(programAdmin);
                firstTimeLoginPanel.Show();

                Close();
            }
            else
            {
                cpLoader.Visibility = Visibility.Collapsed;
                AlertManager.ShowAlertMessage(errorTitle, errorDetail);
            }
        }

Notice I changed the the internal Task to a run a Func<bool> instead of an Action. This way it can return a value bool to indicate the result. Because you are using a lambda function, you can also access variables like errorTitle from both threads, which I am using to pass the details of any error back to the main thread. Instead of sharing variables between threads, you could also change the return type of Task to be a more complex class or a Tuple.

These changes don't really alter the functionality of the code, but in my opinion they make it more readable/maintainable by separating out the main-thread code from the background-thread code.

Keith Stein
  • 6,235
  • 4
  • 17
  • 36
  • Thanks a lot for this, I tried this out, copied the code & set the variables you added and it works like a charm. Example like this is really usefull and easier to understand, I see the change with the bool status, now i have to understand this a bit better to adapt everywhere in my program since a lot of stuff in there interacts with my database. Thanks a lot again for this and the explaining, time to study some more again hehe – Skeiln Oct 18 '19 at 17:14
  • sorry im comming back here in the comment section but I cannot find a PM function on this website, and I thought maybe I can ask this on you quickly rather than making a brandnew post... but could you guide me out here a little please, lets say I have a async Task method (called ExampleX) but i need to access it from another method (no event like the button click) but this method is not async so i cant use await ExampleX() in this method, is going for Dispatcher.Invoke(() => ExampleX()) the best way then instead? Thanks a lot in advance – Skeiln Oct 21 '19 at 03:34
  • You might be better off posting that as its own question. I'm not actually an expert on using `Task`. I use `BackgroundWorker` for my multi-threading and never really got into `async` and `await`. – Keith Stein Oct 21 '19 at 15:27