Updating progressbar from another class in c#

Question

I'm having an issue getting a progressbar to show download progress. The files are downloading without issue, but something is causing my progressbar to not update and I can't figure out why.

I've tried setting the progressBar value manually in the download and wc_DownloadProgressChanged method, but the only place it actually changes is the Form1_Load method.

using System;
using System.Windows.Forms;
using System.Threading;

namespace Launch
{
    public partial class Form1 : Form
    {

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {

            Downloader downloader = new Downloader();

            ThreadStart job = new ThreadStart(downloader.download);
            Thread thread = new Thread(job);
            thread.Start();

        }

        private void ProgressBar_Click(object sender, EventArgs e)
        {

        }

        public void SetProgress(int val)
        {
            progressBar.Value = val;
        }

        public void SetVisible(bool val)
        {
            progressBar.Visible = val;
        }
    }
}

using System;
using System.Data;
using System.Net;
using Newtonsoft.Json;

namespace Launch
{
    class Downloader
    {

        public void download()
        { 
            WebClient client = new WebClient();

            string url = "https://someurl.com/manifest.json";
            string json = client.DownloadString(url);

            DataSet dataSet = JsonConvert.DeserializeObject<DataSet>(json);

            DataTable dataTable = dataSet.Tables["Required"];

            foreach (DataRow row in dataTable.Rows)
            {

                string remoteUri = row["url"].ToString();
                string fileName = row["name"].ToString();

                client.DownloadProgressChanged += client_DownloadProgressChanged;
                client.DownloadFile(remoteUri, fileName);

                Console.WriteLine("Did something with " + remoteUri);

            }
        }

        private void client_DownloadProgressChanged(object sender, DownloadProgressChangedEventArgs e)
        {

            var form = new Form1();
            form.SetProgress(e.ProgressPercentage);

        }

    }
}

Would anyone be able to shed some light on what I'm doing wrong here?

EDIT:

I was able to get this working for the most part using DownloadFileAsync, but the progress bar was bouncing back and forth, I'm assuming because it's trying to calculate the progress for each individual file as bytes are received, so I'd like to get this working with DownloadFile.

The issue I'm now having with DownloadFile is that I have it running as a task but it's skipping all of the files (not downloading any of them, just prints them all out to console super fast).

Here's the code I'm using currently:

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {

            WebClient client = new WebClient();
            client.DownloadProgressChanged += new DownloadProgressChangedEventHandler(client_DownloadProgressChanged);

            string url = "https://someurl.com/manifest.json";
            string json = client.DownloadString(url);

            DataSet dataSet = JsonConvert.DeserializeObject<DataSet>(json);

            DataTable dataTable = dataSet.Tables["Required"];

            foreach (DataRow row in dataTable.Rows)
            {

                string remoteUri = row["url"].ToString();
                string fileName = row["name"].ToString();

                Task.Run(() => {
                    client.DownloadFile(remoteUri, fileName);
                });

                Console.WriteLine("Did something with " + remoteUri);

            }

        }

        private void client_DownloadProgressChanged(object sender, DownloadProgressChangedEventArgs e)
        {

            this.BeginInvoke((MethodInvoker)delegate {
                double bytesIn = double.Parse(e.BytesReceived.ToString());
                double totalBytes = double.Parse(e.TotalBytesToReceive.ToString());
                double percentage = bytesIn / totalBytes * 100;
                label1.Text = "Downloaded ";
                label2.Text = e.BytesReceived.ToString();
                label3.Text = e.TotalBytesToReceive.ToString();
                progressBar.Value = int.Parse(Math.Truncate(percentage).ToString());
            });

        }    

Any ideas?

Answer 1

Use this code below :

    private void Form1_Load(object sender, EventArgs e)
    {
        WebClient client = new WebClient();

        client.DownloadProgressChanged += client_DownloadProgressChanged;
        client.DownloadStringCompleted += client_DownloadStringCompleted;

        Uri url = new Uri("url");
        client.DownloadStringAsync(url);


    }

    void client_DownloadStringCompleted(object sender, DownloadStringCompletedEventArgs e)
    {
        SetVisible(false);
    }

    void client_DownloadProgressChanged(object sender, DownloadProgressChangedEventArgs e)
    {
        SetProgress(e.ProgressPercentage);
    }
    public void SetProgress(int val)
    {
        progressBar.Value = val;
    }

    public void SetVisible(bool val)
    {
        progressBar.Visible = val;
    }

Answer 2

This likely should belong to https://codereview.stackexchange.com/

First of all, do not add a handler in a loop

client.DownloadProgressChanged += client_DownloadProgressChanged;

First time it will be fine, on 2nd item it will be called 2x times, on 3rd time it will be called 3 times, etc. You want to set handler once. Move it just after line:

WebClient client = new WebClient();

Second of all, each time a progress update is fired you are creating a new instance of the form. Create a private variable or property once.

private Form1 form = new Form1();

---

edit:

In case of UI, usually you can modify it only in the dispatcher thread that created it or use marshaling. I would just remove it as our already downloading the string async.

Also, I would not use e.ProgressPercentage, but something like:

Math.Truncate(e.BytesReceived / (double)e.TotalBytesToReceive * 100)