Question

I've got the following class:

public class Terminal : IDisposable
{
    readonly List<IListener> _listeners;

    public Terminal(IEnumerable<IListener> listeners)
    {
        _listeners = new List<IListener>(listeners);
    }

    public void Subscribe(ref Action<string> source)
    {
        source += Broadcast;
        //Store the reference somehow?
    }

    void Broadcast(string message)
    {
        foreach (var listener in _listeners) listener.Listen(message);
    }

    public void Dispose()
    {
        //Unsubscribe from all the stored sources?
    }
}

I've searched for a while and it appears that an argument passed with the ref keyword can't be stored. Trying to add the source argument to a list or to assign it to a field variable doesn't allow it to keep a reference to the actual delegate's original reference; so my questions are:

  • Is there a way to unsubscribe from all the sources without passing their references again?
  • If not, how can the class be changed in order to support it, but still maintain the subscription by passing a delegate through a method?
  • Is it possible to achieve it without using Reflection?
  • Is it possible to achieve it without wrapping the delegate/event in a class and then passing the class as a parameter for the subscription?

Thank you.

EDIT: It appears that without using a Wrapper or Reflection, there's no solution to the given problem. My intention was to make the class as much portable as possible, without having to wrap delegates in helper classes. Thanks everyone for the contributions.

Was it helpful?

Solution

Edit: Ok, that was a bad idea, so back to the basics:

I recommend creating a wrapper class over an Action:

class ActionWrapper
{
    public Action<string> Action;
}

And restructuring your initial class to work with wrappers:

private ActionWrapper localSource;

public void Subscribe(ActionWrapper source)
{
    source.Action += Broadcast;
    localSource = source;        
}

public void Dispose()
{
    localSource.Action -= Broadcast;
}

Now you should get the desired results.

OTHER TIPS

public class Terminal : IDisposable
{
  List<IListener> _listeners;
  List<Action<string>> _sources;

  public Terminal(IEnumerable<IListener> listeners)
  {
      _listeners = new List<IListener>(listeners);
      _sources = new List<Action<string>>();
  }

  public void Subscribe(ref Action<string> source)
  {
      _sources.Add( source );
      source += Broadcast;
  }

  void Broadcast(string message)
  {
      foreach (var listener in _listeners) listener.Listen(message);
  }

  public void Dispose()
  {
      foreach ( var s in _sources ) s -= Broadcast; 
  }
}

I would suggest that the subscription method should return an implementation of a SubscriptionHelper class, which implements IDisposable. A simple implementation would be for SubscriptionHelper to hold a reference to the subscription list and a copy of the subscription delegate; the subscription list itself would be a List<SubscriptionHelper>, and the Dispose method for SubscriptionHelper would remove itself from the list. Note that if the same delegate gets subscribed multiple times, each subscription will return a different SubscriptionHelper; calling Dispose on a SubscriptionHelper will cancel the subscription for which it had been returned.

Such an approach would be much cleaner than the Delegate.Combine/Delegate.Remove method used by the normal .net pattern, whose semantics can get very strange if an attempt is made to subscribe and unsubscribe multi-target delegates.

EDIT:

Yep, my bad - delegates are immutable types, so adding a method to an invocation list will actually create a new delegate instance.

Which leads to an answer no to your question. To unsubscribe the delegate you need to remove your Broadcast method from the delegate's invocation list. This means creating a new delegate and assigning it to the original field or variable. But you cannot access the original once you're out of Subscribe method. Plus there can be other copies of that original field/variable that have your method on the invocation list. And there is no way for you to know about all of them and change there values.

I'd suggest to declare an interface with the event for your purpose. This will be quite flexible approach.

public interface IMessageSource
{
    event Action<string> OnMessage;
}

public class MessageSource : IMessageSource
{
    public event Action<string> OnMessage;

    public void Send(string m)
    {
        if (OnMessage!= null) OnMessage(m);
    }
}

public class Terminal : IDisposable
{
    private IList<IMessageSource> sources = new List<IMessageSource>();

    public void Subscribe(IMessageSource source)
    {
        source.OnMessage += Broadcast;
        sources.Add(source);
    }


    void Broadcast(string message)
    {
        Console.WriteLine(message);
    }

    public void Dispose()
    {
        foreach (var s in sources) s.OnMessage -= Broadcast;
    }
}

Original answer

Is there a particular reason why you pass source delegate as ref? You need this if you, for example, want to return a different delegate from the method.

Otherwise, the delegate is reference type, so you can subscribe to it without passing it as ref...

It's reasonably simple, but there are a few pitfalls. If you store a reference to the source objects, as most of the examples so far have proposed, the object won't be garbage collected. The best way to avoid this is to use an WeakReference, that will allow the GC to work properly.

So, all you have to do is this:

1) Add a list of sources to the class:

private readonly List<WeakReference> _sources = new List<WeakReference>();

2) Add the source to the list:

public void Subscribe(ref Action<string> source)
{
    source += Broadcast;
    //Store the reference 
    _sources.Add(new WeakReference(source));
}

3) And then just implement dispose:

public void Dispose()
{
    foreach (var r in _sources)
    {
        var source = (Action<string>) r.Target;
        if (source != null) 
        {
            source -= Broadcast;
            source = null;
        }
    }


    _sources.Clear();
}

That said, there's also the question of why the Action must be passed as a ref. In the current code, there's no reason for that. Anyway, it doesn't affect the problem or the solution.

Perhaps, instead of trying to store a reference to the delegate, have what calls Subscribe use its reference to the object with the delegate to create actions for the subscription and unsubscription . Its an additional parameter, but its still straightforward.

public void Subscribe(Action<Action<string>> addHandler,Action<Action<string>> removeHandler)
    {
        //Prevent error for possibly being null in closure
        Action<string> onEvent = delegate { };

        //Broadcast when the event occurs, unlisten after (you could store onEvent and remove handler yourself)
        onEvent = (s) => { Broadcast(s); removeHandler(onEvent); };
        addHandler(onEvent);
    }

And an example subscribing.

public event Action<string> CallOccured;

    public void Program()
    {
        Subscribe(a => CallOccured += a, a => CallOccured -= a);
        CallOccured("Hello");
    }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top