What's the most elegant way to swap out events when setting a property?

Question

Doing component-based development, I find myself doing this fairly often:

public class SomeClass
{
    SomeOtherClass foo;

    public SomeOtherClass Foo
    {
        get { return foo; }
        set {
            if (value != foo) {
                if (value != null) {
                    // subscribe to some events
                    value.SomeEvent += foo_SomeEvent;
                }

                if (foo != null) {
                    // unsubscribe from subscribed events
                    foo.SomeEvent -= foo_SomeEvent;
                }

                foo = value;
            }
        }
    }

    void foo_SomeEvent(object sender, EventArgs e)
    {
        // do stuff
    }
}

Is there any more elegant way to do this event "swapout"?

(Of course, the whole issue could be avoided if foo was immutable, but then I wouldn't get any visual designer support.)

Answer 1

I do think your current implementation is perfectly acceptable. It is very clear and easy to follow.

---

If you want to shorten the code, since you're doing this a lot, you could make a method that does it:

private void SwapEventHandler<T>(T a, T b, Action<T> subscribe, Action<T> unsubscribe)
    where T : class
{
    if (a != null) 
        subscribe(a);
    if (b != null)
        unsubscribe(b);
}

You could then write:

if (value != foo) 
{
    SwapEventHandler(value,foo, (o) => o.SomeEvent += foo_SomeEvent, (o) => o.SomeEvent -= foo_SomeEvent );
    foo = value;
}

Answer 2

What you are doing is fine, but I would generally prefer the convention of unsubscribing old event handlers before subscribing the new ones, just to avoid any potential "overlap" where both objects could try to handle the same event if it fired from another thread in between the calls.

For a marginal improvement, you could dispense with unnecessary braces to make the code more compact and "tidier" (where "tidier" is in the eye of the beholder).

set
{
    if (value != foo)
    {
        if (foo != null)
            foo.SomeEvent -= foo_SomeEvent;
        if (value != null)
            value.SomeEvent += foo_SomeEvent;

        foo = value;
    }
}

If you make it illegal to use null references (e.g. by using a reference to a "Null Foo Object" instead of a null) then you could dispense with the ifs entirely:

set
{
    if (value != foo)
    {
        foo.SomeEvent -= foo_SomeEvent;
        value.SomeEvent += foo_SomeEvent;
        foo = value;
    }
}

In some cases where you have many properties working with similar objects/events you could possibly also implement the swap code in a (generic) helper method so that you have the implementation in one place and your properties all just call a shared helper method. This would only be of benefit if you could share one implementation for many properties though:

set
{
    Helpers.SetValueAndResubscribeFooSomeEvent(ref foo, value);
}

Answer 3

If you dont know which events has been registered on the event, and want to clear it completly, you can do the following:

public class SomeOtherClass
{
    public event EventHandler SomeEvent;

    public void ClearSomeEvent()
    {
        foreach (EventHandler e in SomeEvent.GetInvocationList())
        {
            SomeEvent -= e;
        }
    }
}

And in SomeClass.Foo property setter:

if (foo != null)
{
    // unsubscribe from subscribed events                    
    foo.ClearSomeEvent();                
}

If you do know the subcribed delegate, youre current solution is fine.