Using custom events in an interface 2

[BobRodes]

I've been working with a piece of code, and started this thread: thread732-1539660. Now I'm having a problem adding an event with a custom set of arguments. The error I'm getting is An object reference is required for the non-static field, method, or property 'ConsoleClient.Program.de_OnExportFinished(DataExport.DataExporter, DataExport.DEEventArgs)'
Here's the code, with relevant (at least, what I think are relevant) lines in red. Can anyone enlighten me? TIA -Bob

[COLOR=red]//DataExporter.cs[/color]
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data;
using DataExport; 

namespace DataExport
{
        [COLOR=red]public delegate void DataExporterEvent (DataExporter sender, DEEventArgs deargs);[/color]

    interface iDataMethods
{
        void Export(string XMLFilePath);
        int AddRecord(string RecordText);
        bool AllowExport 
        {
            get;
            set;
        }
        [COLOR=red]event DataExporterEvent OnExportFinished;[/color]
    }

[COLOR=red]    public class DEEventArgs: EventArgs
    {
        private int es;
        private string fname;
        
        public DEEventArgs(int eStatus, string fName)
        {
            es = eStatus;
            fname = fName;
        }

        public int ExportStatus
        {
            get { return es; }
        }

        public string FileName
        {
            get { return fname; }
        }

    }
[/color]
    
     public class DataExporter : iDataMethods  //implements the iDataMethods interface
    {
        private List<string> addedrecords = new List<string>();  //generic List object, will hold a single
        //string field of data.  In real life, this would either be an object, a struct, or perhaps a 
        //semicolon-delimited string that could be parsed into multiple fields.
        private bool exportallowed;
        private static DataExporter instance = null;  //Holds a private reference to the singleton instance.
        private static readonly object locker = new object();  //Used to lock a block of code

        [COLOR=red]public event DataExporterEvent OnExportFinished;[/color]
        
        private DataExporter() { }  //overriding the default constructor to make it private.  Private constructor 
        //requires instantiation via a method or property, in this case the Instance property.

        public static DataExporter Instance  //self-reference
        {
            get
            {
                lock (locker) //locking makes the code in the brackets execute atomically.  This code is the basic
                //singleton pattern: check for an existing instance, if not found create it, return result.
                {
                    if (null == instance)
                    {
                        instance = new DataExporter(); //self-reference
                    }
                    return instance;
                }
            }
        }
        
        public bool AllowExport
        {
            get { return exportallowed; }
            set { exportallowed = value; }
        }

        //Export transfers the present contents of the List object (addedrecords) into a DataSet (ds), writes
        //the result to an XML file, and clears the List object.
        public void Export(string XMLFilePath) 
        {
            if (!exportallowed) 
            {
                [COLOR=red]DEEventArgs deargs = new DEEventArgs(2, XMLFilePath);
                if (OnExportFinished != null) { OnExportFinished(this, deargs); }[/color]
            }
            try
            {
                DataSet ds = new DataSet();
                ds.Tables.Add();
                ds.Tables[0].Columns.Add("TheField", typeof(string));
                foreach (string ar in addedrecords)
                {
                    ds.Tables[0].Rows.Add(ar);
                }
                ds.WriteXml(XMLFilePath);
                addedrecords.Clear();
                [COLOR=red]DEEventArgs deargs = new DEEventArgs(1, XMLFilePath);
                if (OnExportFinished != null) { OnExportFinished(this, deargs); }[/color]
            }
            catch
            {
               [COLOR=red] DEEventArgs deargs = new DEEventArgs(0, XMLFilePath);
                if (OnExportFinished != null) { OnExportFinished(this, deargs); }[/color]
            }
        }

        public int AddRecord(string RecordText)
        {
            try
            {
                addedrecords.Add(RecordText);
                return 1;
            }
            catch
            {
                return 0;
            }
        }
    }
}


[COLOR=red]Program.cs[/color]
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using DataExport;

namespace ConsoleClient
{
    class Program
    {
        static void Main(string[] args)
        //creating two threads that use the DataExporter singleton, to test for thread safety.  The parameters
        //"1" and "2" carry over into the XML files.
        {
            Thread t1 = new Thread(new ParameterizedThreadStart(doStuff));
            Thread t2 = new Thread(new ParameterizedThreadStart(doStuff));

            t1.Start("1");
            t2.Start("2");
        }

        static void doStuff(Object tPar)
        {
            int eVal;
            string fileName = "c:\\Temp\\myFile" + tPar + ".xml";
            DataExporter de = DataExporter.Instance;
[COLOR=red]            de.OnExportFinished+=new DataExporterEvent(de_OnExportFinished);
//*****ERROR HAPPENS HERE******[/color]
            de.AllowExport = true;
            lock (de) //Locking the block ensures thread safety.  Removing the lock causes all of the line items
            //in both threads to be written to each of the two xml files, when we want one thread for each file.
            {
                for (int i = 1; i <= 20; i++)
                {
                    eVal = de.AddRecord("Thread " + tPar + "Record " + Convert.ToString(i));
                    if (0 == eVal)
                    {
                        Console.WriteLine("AddRecord didn't work");
                        return;
                    }
                    Thread.Sleep(100);
                }
                de.Export(fileName);
                Console.WriteLine("strike a key when ready...");
                Console.ReadKey(true);
            }
        }

[COLOR=red]        private void de_OnExportFinished(DataExporter sender, DEEventArgs deargs)
        {
            switch (deargs.ExportStatus)
            {
                case 0:
                    Console.WriteLine("Export didn't work");
                    break;
                case 2:
                    Console.WriteLine("Export function disabled");
                    break;
                default:
                    Console.WriteLine("Export to " + deargs.FileName + " Successful.");
                    break;
            }
        }
[/color]    }
}

 

[BobRodes]

Ok, I'm seeing that the reason I'm getting the error here is because doStuff is static. But it has to be, because the thread requires it. I'll keep digging...

 

[BobRodes]

Ok, I made the de_DataExportFinished event handler static, and that solved the problem. I'm not sure what the ramifications are of not having one handler per instance, but since the handler is doing the same thing for each instance it doesn't look to me like this will be a problem.

 

[BobRodes]

All right, one more strange problem here. When I run this code, my de_OnExportFinished switch statement runs correctly on the first thread, but runs twice on the second thread, with the result that I get the message twice on the second thread. Can anyone see a reason for that offhand?

 

[jmeckley]

When I run this code, my de_OnExportFinished switch statement runs correctly on the first thread, but runs twice on the second thread, with the result that I get the message twice on the second thread.

yes, that's correct.
the handler is static, and you register it twice.
de.OnExportFinished+=new DataExporterEvent(de_OnExportFinished);
is called twice.

one thing to do is to unregister the event when your finished
de.OnExportFinished-=new DataExporterEvent(de_OnExportFinished);
but this won't help your threading issue. you can still have race conditions. I think this ties back to the original challenge of a singleton exporter vs instance exporting with a static factory.


Jason Meckley
Programmer
Specialty Bakers, Inc.

 

[BobRodes]

Why is it called twice when the second thread executes, and only once when the first thread executes though?

 

[jmeckley]

the same would be true if you removed the threading.
the first thread registers a handler for the event and executes. then the second thread registers another handler (in this case the same one) and executes. the 2nd time around the event fires 2 registered handlers.

Jason Meckley
Programmer
Specialty Bakers, Inc.

 

[BobRodes]

All right. I have slain the savage beast. Jason, from what I've been able to read, the lock prevents race conditions on the thread. Is that incorrect?

Here are the changes I made to Main and doStuff (I didn't have to change anything else) to get it working:

    class Program
    {
        static void Main(string[] args)
        {
            Thread t1 = new Thread(new ParameterizedThreadStart(doStuff));
            Thread t2 = new Thread(new ParameterizedThreadStart(doStuff));
            t1.Name = "Thread1";
            t2.Name = "Thread2";
            t1.Start("1");
            t2.Start("2");
            t1.Join();
            t2.Join();
            Console.WriteLine("strike a key when ready...");
            Console.ReadKey(true);
         }

         static void doStuff(Object tPar)
        {
            int eVal;
            string fileName = "c:\\Temp\\myFile" + tPar + ".xml";
            DataExporter de = DataExporter.Instance;
 
            de.AllowExport = true;

            lock (de) 
            {

                for (int i = 1; i <= 20; i++)
                {
                    [...]
                }
                de.OnExportFinished += new DataExporterEvent(de_OnExportFinished);
                de.Export(fileName);
                de.OnExportFinished -= new DataExporterEvent(de_OnExportFinished);
            }
        }

I made the following changes:

1. Placed the event registration in the lock block.
2. Added Jason's code to unregister the event.
3. Moved the Console ReadKey from doStuff to Main.
4. Added the Join logic to prevent "strike a key when ready" from appearing before the event messages.

Thanks for your help, Jason!

Bob

 

[jmeckley]

the lock prevents race conditions on the thread. Is that incorrect?

to the best of my knowledge, yes. However, you probably have a better understand of locking than me.

Jason Meckley
Programmer
Specialty Bakers, Inc.

 

[BobRodes]

Double checking here. Yes that is incorrect or yes that is correct?

What I know about locking in C# I've read in the last few days. The lock statement is a shorthand for use of the System.Threading.Monitor class. It's my understanding that anything in a lock block will execute atomically with relationship to the current thread.

 

[jmeckley]

yes that is correct, sorry for the confusion

The lock statement is a shorthand for use of the System.Threading.Monitor class

that I did not know.

Jason Meckley
Programmer
Specialty Bakers, Inc.

 

[BobRodes]

Just read it the other day. Here:

http://msdn.microsoft.com/en-us/library/hf5de04k.aspx