views:

141

answers:

3

I'm unsure as to what is the best approach for passing events down the line to parent classes and in need of some feedback.

The example code below tries to illustrate what I want to achieve.

namespace test {
public delegate void TestCompletedEventHandler(object sender, 
    TestCompletedEventArgs e);

    public class Manager {
        CarList m_carlist = null;

        public CarList Cars {
            get { return m_carlist; }
            set { m_carlist = value; }
        }

        public Manager() {
            Cars = new CarList(this);
        }

        public void Report(bool successfull) {
            //...
        }
    }

    public class CarList : List<Car> {
        protected internal event TestCompletedEventHandler 
            Car_TestCompleted = null;

        protected readonly Manager m_manager = null;

        public Manager Manager {
            get { return m_manager; }
        }

        public CarList(Manager manager) {
            m_manager = manager;
        }

        public void Test() {
            foreach(Car car in this) {
                bool ret = car.Test();
                manager.Report(ret);
            }
        }

        public void Add(Car car) {
            //Is this a good approach?
            car.TestCompleted += 
                new TestCompletedEventHandler(Car_TestCompleted_Method);
            base.Add(car);
        }

        private void Car_TestCompleted_Method(object sender, 
            TestCompletedEventArgs e) 
        {
            if(Car_TestCompleted != null) Car_TestCompleted(sender, e);
        }
    }

    public class Car {
        protected internal event TestCompletedEventHandler 
            TestCompleted = null;

        public bool Test() {
            //...

            if(TestCompleted != null) TestCompleted(this, 
                new TestCompletedEventArgs())
        }
    }

    public class TestCompletedEventArgs : EventArgs {
        //...
    }
}

using test;
Manager manager = new Manager();
manager.Cars.Car_TestCompleted += 
    new TestCompletedEventHandler (Car_TestCompleted_Method);
manager.Cars.Test();

Another more specific example:

//Contains DataItems and interfaces for working with them
class DataList
{
 public List<DataItem> m_dataitems { get; set; }
 public TestManager m_testmanager { get; set; }
 // ...
}

class DataItem
{
 // ...
}

//A manager class for running tests on a DataList
class TestManager 
{
 public List<TestSource> m_sources { get; set; }
 public WorkerManager m_workermanager { get; set; }
 // ...
}

//A common interface for Tests
abstract class TestSource
{
 public event EventHandler<EventArgs<object>> Completed = null;
 protected TestManager m_owner { get; set; }

 public abstract void RunAsync();
 // ...
}

//A test
class Test1 : TestSource
{
 public virtual void RunAsync()
 {
  //Add commands
  //Run workers
  //Report progress to DataList and other listeners (like UI)

  //Events seem like a bad approach since they need to be forwarded through many levels of abstraction
  if(Completed != null) Completed(this, new EventArgs<object>(null));
 }
 // ...
}

//Manages a number of workers and a queue of commands
class WorkerManager
{
 public List<MyWorker> m_workers { get; set; }
 public Queue<Command> m_commands { get; set; }
}

//Wrapper for BackgroundWorker
class MyWorker
{
 // ...
}

//Async command
interface Command
{
 // ...
}
+1  A: 

It wouldn't make sense to just have each car call an event which calls an event on the parent list. I would do it more like this:

namespace test {
    public delegate void TestCompletedEventHandler(object sender, 
    TestCompletedEventArgs e);

    public class Manager {
        CarList m_carlist = null;

        public CarList Cars {
            get { return m_carlist; }
            set { m_carlist = value; }
        }

        public Manager() {
            Cars = new CarList(this);
        }

        public void Report(bool successful) {
            //...
        }
    }

    public class CarList : List<Car> {
        protected readonly Manager m_manager = null;
     protected List<Action<object, TestCompletedEventArgs>> delegatesList = new List<Action<object, TestCompletedEventArgs>>();

        public Manager Manager {
            get { return m_manager; }
        }

        public CarList(Manager manager) {
            m_manager = manager;
        }

        public void Test() {
            foreach(Car car in this) {
                bool ret = car.Test();
                manager.Report(ret);
            }
        }
        public void Add(TestCompletedEventHandler e) {
            foreach (Car car in this) {
                car.OnTestCompleted += e;
            }
      delegatesList.Add(e);
        }
        public void Add(Car car) {
     foreach(Action a in delegatesList)
     {
         car.OnTestCompleted += a;
     }
            base.Add(car);
        }
    }

    public class Car {
        protected internal event TestCompletedEventHandler OnTestCompleted = null;

        public bool Test() {
            //...
            if (OnTestCompleted != null) OnTestCompleted(this, new TestCompletedEventArgs());
        }
    }

    public class TestCompletedEventArgs : EventArgs {
        //...
    }
}

using test;
Manager manager = new Manager();
Manager.Cars.Add(new Car());
manager.Cars.Add(new Car());
manager.Cars.Add(new Car());
manager.Cars.Add((sender, args) => 
{
    //do whatever...
})
manager.Cars.Test();
manager.Cars.Add(new Car());
RCIX
What if I add more cars after the EventHandler is set using "manager.Cars.Add((sender, args)"?
magix
Hmm.... Good question. I might be able to come up with something better, hang on. Here it is, edited in.
RCIX
This of course won't handle removing events, for that you need to add a Remove method similar to the Add one only it does the opposite.
RCIX
+1  A: 

It seems reasonable, but I'm not really sure what the use case is and how this would be used.

You've got a strong concept of containment going on, but I'm not really sure why. Also, it's kind of weird that the CarList 'sort of' seems to have ownership of the individual cars.

Additionally, I don't know why Test() on the Car class would both return a result and raise an event. It seems like you're having two different paths to return the same data. And the Manager class seems completely redundant with the CarList class at first glance.

What is the problem you're actually trying to solve here? That might help me with defining a good solution to it.

kyoryu
I've updated the origianal question with a more specific example.Most of the "heavy" code is run in async worker threads and they need to report up the aggregation-chain when they complete running/change status/on progress etc.
magix
+2  A: 

I think you may have just over implemented this a bit... It looks like you are trying to use async operations. Even if you are using sync operations though, typically you'd just use callback methods instead of events in a case like this...

Here is an example of things to change to use callbacks here:

//new delegate
public delegate void CarReportCallback(Car theCar, bool result);

//in the Manager class, make report conform to delegate's signature
public void Report(Car theCar, bool result)
{
    //do something, you know which car and what the result is. 
}

//in the CarList class pass a reference to the report method in
public void Test() 
{
    foreach(Car car in this)
    {
        car.Test(manager.Report);
    }
}

//in the Car class use the delegate passed to invoke the reporting 
public void Test(CarReportCallback callback)
{
    //... do stuff
    callback(this, isTestCompleted);
}
Stephen M. Redd