Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Event Handlers in Constructors - Is it Possible or Even Wise?

Tags:

.net

I have an object that can take anywhere from a split second to a couple of minutes to initialize. The reason being is that the constructor retrieves data from a webservice that could be a few kilobytes to several megabytes, and depending on the user's connection speed performance may vary greatly. For that reason I am wanting to put events in that will handle progress notification.

Here is my question: Can I put event handlers in the constructor or should this type of action be done with a Load method?

For example:

public class MyObject
{    
 public event EventHandler<UpdateLoadProgressEventArgs> UpdateLoadProgress;    

 public MyObject(int id)
 {
   Background worker bgWorker = new BackgroundWorker();
   bgWorker.DoWork += delegate(object s, DoWorkEventArgs args)
   {
        //load data and update progress incrementally
        UpdateLoadProgress(this, new UpadteLoadProgressEventArgs(progressValue));

        Result = someValue;         
   }
   bgWorker.RunWorkAsync();

 } 

 public int Result
 {
  get;
  set;
 }

} 

However when I try to tie the event handlers to the constructor they are always null when being called:

MyObject o = new MyObject(1);
o.UpdateLoadProgress += new EventHandler<EventArgs>(o_UpdateLoadProgress);

I assume this happens because I wire up the events after the constructor. The only alternative I see is creating a Load method that does the work of the constructor. The downside is that anyone that uses this class must know to call Load before trying to access Result (or any other property).

EDIT: Here is the final solution:

MyObjectBuilder Class

public class MyObjectBuilder
    {
        public event ProgressChangedEventHandler ProgressChanged;

        public MyObject CreateMyObject()
        {
            MyObject o = new MyObject();
            o.Load(ProgressChanged);

            return o;
        }
    }

MyObject Class

public class MyObject 
    {
        public int Result { get; set;}

        public void Load(ProgressChangedEventHandler handler)
        {
            BackgroundWorker bgWorker = new BackgroundWorker();
            bgWorker.WorkerReportsProgress = true;
            bgWorker.ProgressChanged += handler;

            bgWorker.DoWork += delegate(object s, DoWorkEventArgs args)
            {
                for (int i = 0; i < 100; i++)
                {
                    Thread.Sleep(10);
                    Result = i;

                    bgWorker.ReportProgress(i);
                }
            };
            bgWorker.RunWorkerAsync();                      
        }
    }

Program Class

class Program
    {
        static void Main(string[] args)
        {
            MyObjectBuilder builder = new MyObjectBuilder();
            builder.ProgressChanged += new ProgressChangedEventHandler(builder_ProgressChanged);        

            MyObject o = builder.CreateMyObject();
            Console.ReadLine();
        }

        static void builder_ProgressChanged(object sender, ProgressChangedEventArgs e)
        {
            Console.WriteLine(e.ProgressPercentage);
        }
    }
like image 505
Blake Blackwell Avatar asked Aug 04 '10 12:08

Blake Blackwell


2 Answers

Another option would be to pass the event handlers into the constructor too, of course.

Personally I try to avoid doing something like this within a constructor. Creating a new object shouldn't generally start background tasks, IMO. You may want to put it into a static method instead - which can call a private constructor, of course.

You could also split up your class into two - a builder which gets everything ready (e.g. the events), and then an "in-flight or completed" class, which has the Result property. You'd call Start or something similar on the first class to get an instance of the second.

like image 51
Jon Skeet Avatar answered Sep 20 '22 10:09

Jon Skeet


Is this possible? Maybe. Is it wise? No.

The most obvious reason is that you aren't guaranteed that the background thread will not execute before the event handlers are wired up after the object's construction. Or worse, some of the event handlers may be subscribed while others are not.

like image 25
villecoder Avatar answered Sep 20 '22 10:09

villecoder