[adn258]

So some have suggested Using using with a background worker may be good, but I'm concerned about the events etc, all being disposed correctly or if this is a good practice at all example I'm thinking of using is in my code below. IS this acceptable and will DoWork also be disposed in something like this? Thanks in advance guys.

 using (BackgroundWorker bw = new BackgroundWorker())
            {
                bw.DoWork += delegate
                {
                    string[] imagefiles = new string[this.LstBxFilesToConvert.Items.Count];
                    this.LstBxFilesToConvert.Items.CopyTo(imagefiles, 0);

                    ImageConversion ic = new ImageConversion(this.TxtBxOutputLocation.Text, this.ImageConversionFormat);
                    DoGUIAction(() => this.progressBar1.Maximum = imagefiles.Length);
                    if (this.ChkBxUseParralel.Checked)
                    {
                        Parallel.ForEach(imagefiles, ifile =>
                        {
                            try
                            {
                                ic.ConvertImages(ifile);
                            }
                            catch
                            {
                                failedconversions++;
                            }
                            DoGUIAction(() => this.progressBar1.Value++);
                        });
                    }
                    else
                    {
                        foreach (string ifile in imagefiles)
                        {
                            try
                            {
                                ic.ConvertImages(ifile);
                            }
                            catch
                            {
                                failedconversions++;
                            }
                            DoGUIAction(() => this.progressBar1.Value++);
                        }
                    }
                   
                };
                bw.RunWorkerAsync();
            }

[[email protected]]

I wouldn't use the BackgroundWorker in a using statement. bw.RunWorkerAsync(); will return almost immediately as pretty much all it does is queue your work to the thread pool. So, in your code, the BackgroundWorker will be disposed of before it has finished executing your work.

Now, while this won't strictly cause any problems for the BackgroundWorker class specifically, disposing of object whilst it is still in use is not a habit I would advise getting into.

Further, a large part of the reason why calling Dispose() on a BackgroundWorker object whilst it is still in use doesn't cause a problem, is because the only thing Dispose() does is remove the BackgroundWorker from its ISite.Container (if it is in one), and then raise the Disposed event (if someone is subscribed to it).

However, even if Dispose() doesn't really do anything of any great importance, I am usually one of those people that would still try to call it. I think it is a good habit to get in, and it avoids the need for the object to be finalized.

However, in a case where the object is devoid of resources, and you are finding it awkward to call Dispose(), you could just leave it up to the garbage collector to finalize the object, and only if you conclusively measured that this was causing a problem, would you start to bend over backwards to call Dispose().

Although, in this case, just doing this:

bw.RunWorkerCompleted += (o, e) => (o as BackgroundWorker).Dispose();

seems like an easy way to ensure the disposal.

EDIT:

Also, be careful not to access UI controls from the DoWork event handler method. You seem to be accessing a number of controls from it in your code above. Also, don't forget you can use the BackgroundWorker object's ProgressChanged event to report progress, rather than invoking manually (which I am assuming DoGUIAction() does)

This post has been edited by [email protected]: 08 December 2012 - 04:49 AM

[adn258]

So Like in my program I'm using now

BackgroundWorker bw;
                
        public Form1()
        {
            InitializeComponent();
            InitListView();
            bw = new BackgroundWorker();
            bw.DoWork += new DoWorkEventHandler(bw_DoWork);
            bw.RunWorkerCompleted += new RunWorkerCompletedEventHandler(bw_RunWorkerCompleted);

Meaning that that object never gets disposed which I don't necessarily want it too because The work that it does I want to be used throughout the life of the program. If I were to dipose this the user couldn't use the code in the bw methods and the worker wouldn't run. So is it bad to NOT dispose of this object; does it make any difference?

This post has been edited by adn258: 01 August 2012 - 05:25 PM

[[email protected]]

In the code you posted there, I don't think it would make any noticeable difference whatsoever if you don't dispose of the BackgroundWorker, as Dispose() won't even do anything apart from prevent finalization. The garbage collector will happily finalize the object for you if you don't call Dispose(), which may technically cause a small performance hit to your application, but that will be way to small to even think about.

However, you are using the BackgroundWorker as an instance field in a form, so why don't you either:

1) Drag and drop the BackgroundWorker from the toolbox instead, meaning the Dispose() method will be called for you automatically when the form is disposed.

2) Or, use the BackgroundWorker as you have done in that code, and manually call Dispose() on the BackgroundWorker in the form's Dispose(bool) method.

I said don't worry yourself about calling Dispose() if it is awkward to do so (and therefore may harm the readability and simplicity of the code), and you know/have measured that not calling it doesn't cause a performance issue. In the code you posted there, it is very easy to ensure the BackgroundWorker is disposed of deterministically without harming the simplicity of the code at all, so you should make sure you call Dispose(), in my opinion.

We are getting into the realms of opinion rather than fact with that last statement though

This post has been edited by [email protected]: 02 August 2012 - 02:59 AM

[adn258]

[email protected], on 02 August 2012 - 02:27 AM, said:

In the code you posted there, I don't think it would make any noticeable difference whatsoever if you don't dispose of the BackgroundWorker, as Dispose() won't even do anything apart from prevent finalization. The garbage collector will happily finalize the object for you if you don't call Dispose(), which may technically cause a small performance hit to your application, but that will be way to small to even think about.

However, you are using the BackgroundWorker as an instance field in a form, so why don't you either:

1) Drag and drop the BackgroundWorker from the toolbox instead, meaning the Dispose() method will be called for you automatically when the form is disposed.

2) Or, use the BackgroundWorker as you have done in that code, and manually call Dispose() on the BackgroundWorker in the form's Dispose(bool) method.

I said don't worry yourself about calling Dispose() if it is awkward to do so (and therefore may harm the readability and simplicity of the code), and you know/have measured that not calling it doesn't cause a performance issue. In the code you posted there, it is very easy to ensure the BackgroundWorker is disposed of deterministically without harming the simplicity of the code at all, so you should make sure you call Dispose(), in my opinion.

We are getting into the realms of opinion rather than fact with that last statement though

OKAY I'M MISSING A FEW THINGS HERE sorry lol:
Right on well the thing is and maybe you have an idea about that; ok so in the background worker if I call dispose in the workercompleted the user would NOT be able to call the background worker again unless I created another instance of that when the user clicks a button or something in which case there's no use in constructing the background worker in the constructor anyway right?

Meaning correct me if I'm wrong since this is in the constructor it NEVER gets disposed until the form closes or dispose is called but this is on the main form of the application meaning it's never disposed and I want the user to be able to use the background worker multiple times for conversions NOT JUST ONCE and then if I dispose it I'd have to create new instances again.

I COULD create the backgroundworker instance within a button click and all the Backgroundworker methods there but then what's the point or maybe I should be doing that instead of the constructor? Also correct me if I'm wrong when you drag and drop a background worker instances this is also being created with the constructor of the form and is essentially the same thing as the above no?

This post has been edited by adn258: 02 August 2012 - 11:16 AM

[[email protected]]

Quote

worker if I call dispose in the workercompleted the user would NOT be able to call the background worker again unless I created another instance of that when the user clicks a button

If you call Dispose() on the BackgroundWorker in your code, you will be able to continue to use the BackgroundWorker with no problems at all.

However, the point is that that is purely an implementation detail of the BackgroundWorker class. It just so happens that the Dispose() method doesn't do much in that particular class, and therefore the BackgroundWorker can continue being used after Dispose() has been called. You obviously shouldn't rely on that fact though, and you shouldn't be calling Dispose() on objects until you have finished with them, even if you can technically continue using the objects after calling Dispose() in the current implementation of the object; it just doesn't make sense to do so.

Quote

I COULD create the backgroundworker instance within a button click and all the Backgroundworker methods there but then what's the point or maybe I should be doing that instead of the constructor?

It's perfectly fine reusing the BackgroundWorker as you are doing.

Quote

Also correct me if I'm wrong when you drag and drop a background worker instances this is also being created with the constructor of the form and is essentially the same thing as the above no?

Dragging and dropping the BackgroundWorker onto the form will be (almost) identical to your code, except Dispose() will be called for you when the form is disposed of.

Quote

since this is in the constructor it NEVER gets disposed until the form closes

In your code, unless you call Dispose() on the BackgroundWorker explicitly, it won't be disposed (even when the form closes) until the garbage collector finalizes it.

Quote

...but this is on the main form of the application meaning it's never disposed

Also, the main form will still be disposed of when you close it (somewhat irrelevantly, admittedly). Anyway, you never know, you may want to reuse that form as a non main form in another project .

Conclusion

I don't really understand the problem. You are getting overly worried over something that is really too small to worry about . Just keep your code as it is, reuse the BackgroundWorker as many times as you wish, and call Dispose() on the BackgroundWorker in the Dispose(bool) method of the form if you want to be consistent, and dispose of it. If you don't dispose of it, don't worry unduly about it as it shouldn't cause any problems at all.

This post has been edited by [email protected]: 02 August 2012 - 12:43 PM
Reason for edit:: Fixed bold tag

[Curtis Rutland]

Quote

I don't really understand the problem. You are getting overly worried over something that is really too small to worry about

QFT.