Having children call parent's public events with data generated from a protected event

Question

I don't know that my title describes the code I want reviewed. Hopefully the code is explanatory.

I have an abstract parent class with two children. The parent listens to some hardware notifications and, when the appropriate hardware is connected/removed/fried in butter etc.., raises an event to its children who perform some processing with the data from the change.

The children then raise a public event on the parent with the processed data, which is what I'm unsure of. Here are the (stripped down) classes.

public abstract class DeviceDetector<TEventArgs>
{
    public event Action<TEventArgs> DeviceArrived;
    public event Action<TEventArgs> DeviceRemoved;
    protected void OnDeviceChange(TEventArgs e)
    {
         if (e.Arrival && DeviceArrived != null)
             DeviceArrived(e);
         if (!e.Arrival && DeviceRemoved != null)
             DeviceRemoved(e);
    }

    protected event Action<string> DeviceArrival;
    protected event Action<string> DeviceRemoval;

}

Each of the two children is defined as follows:

public class SomeDescriptiveChildName: DeviceDetector<DeviceChild1EventArgs>
{
    public SomeDescriptiveChildName()
    {
        this.DeviceArrival += new Action<string>(HandleProtectedEvent);
        this.DeviceRemoval += new Action<string>(HandleProtectedEvent);
    }
    private void HandleProtectedEvent(string e)
    {
         //Class names have been changed to protect the innocent.
         DeviceChild1EventArgs e = new DeviceChild1EventArgs(); 
         base.OnDeviceChange(e);
    }
}

public class AnotherDescriptiveChildName: DeviceDetector<DeviceChild2EventArgs>
{
    public AnotherDescriptiveChildName()
    {
        this.DeviceArrival += new Action<string>(HandleProtectedEvent);
        this.DeviceRemoval += new Action<string>(HandleProtectedEvent);
    }
    private void HandleProtectedEvent(string e)
    {
         DeviceChild2EventArgs e = new DeviceChild2EventArgs(); 
         base.OnDeviceChange(e);
    }
}

The DeviceChild1EventArgs and DeviceChild2EventArgs are classes inheriting from EventArgs to represent the data from each child.

My question is this: Is there a better way to represent the data passed to the children through DeviceArrival and DeviceRemoval (in this example I used string, it's not a string) without the use of generics etc...

To me this seems somewhat like overkill, but it's the only way I could think of doing this.

Answer 1

The events in the base class don't follow the standards:

public event Action<TEventArgs> DeviceArrived;
public event Action<TEventArgs> DeviceRemoved;

protected event Action<string> DeviceArrival;
protected event Action<string> DeviceRemoval;

I'd be expecting something like this:

public event EventHandler<TEventArgs> AfterDeviceArrived;
public event EventHandler<TEventArgs> AfterDeviceRemoved;

protected event EventHandler<SomeEventArgsClass> BeforeDeviceArrived;
protected event EventHandler<SomeEventArgsClass> BeforeDeviceRemoved;

Couple points:

• Events can be declared with any delegate type (Action<T> works), but the convention is to use an EventHandler or EventHandler<TEventArgs> delegate.
• Convention for naming events that come in pairs (raised before and after such or such "thing" happens) is to prefix the event names with "Before" and "After", and use past tense as you've done.
• OnDeviceChange should be called OnDeviceChanged.. but it's not clear why the method needs to raise both ...Arrived and ...Removed events without [apparently] doing anything in-between.

---

A base class shouldn't know, or care, about whether it has derived classes or not. You've got that right, but a derived class that's listening for events raised in the base class, doesn't make sense to me.

I don't think events are the appropriate way to accomplish what you're trying to do here. That said I'm not sure exactly what you're trying to accomplish (trimmed code does that ;) - but I'd venture to suggest templated methods instead of events.

In the base class:

// todo: remove notion of "event args"
protected abstract void OnDeviceArrived(TEventArgs e);
protected abstract void OnDeviceRemoved(TEventArgs e);

The base class can call these methods even though they're not implemented - abstract methods must be implemented in the derived classes for the code to even compile.

So instead of this:

     if (e.Arrival && DeviceArrived != null)
         DeviceArrived(e);
     if (!e.Arrival && DeviceRemoved != null)
         DeviceRemoved(e);

You just have that:

    if (e.Arrival) 
    {
        OnDeviceArrived(e);
    }
    else
    {
        OnDeviceRemoved(e);
    }

And the derived classes have the implementations:

protected override void OnDeviceArrived(TEventArgs e)
{
    // ...
}

protected override void OnDeviceRemoved(TEventArgs e)
{
    // ...
}