55

I find myself doing this sort of thing quite often:-

 EventHandler eh = null;  //can't assign lambda directly since it uses eh
 eh = (s, args) =>
 {
     //small snippet of code here

     ((SomeType)s).SomeEvent -= eh;
 }
 variableOfSomeType.SomeEvent += eh;

Basically I only want to attach an event handler to listen for one shot from the event, I no longer want to stay attached after that. Quite often that "snippert of code" is just one line.

My mind is going a bit numb, I'm sure there must be something I can do so I don't need to repeat all this overhead. Bear in mind that EventHandler may well be EventHandler<T>.

Any ideas how I can tidy up the repeative part of the code and just leave the snippet in a Lambda?

AnthonyWJones
  • 187,081
  • 35
  • 232
  • 306
  • It smells, I agree. Looking forward to see the answers. ;) – Lucero Jan 27 '10 at 22:01
  • What about attaching a permanent event handler which invokes "one shot event handlers" added to a queue? – dtb Jan 27 '10 at 22:01
  • Interesting; we did something similar in PushLINQ; but for re-use there are problems; lambdas can't pick out events, and without the event, type inferencing is a pain - and without type inferencing the handler can't be a lambda... fun. – Marc Gravell Jan 27 '10 at 22:04
  • Out of curiosity, why not just use a member variable boolean flag that gets set once the event fires, then do "if (flag) return;"? – Josh Kodroff Jan 27 '10 at 22:07
  • Should have added above: The idea of the lambda expression removing itself as the event handler seems very spaghetti-ish. – Josh Kodroff Jan 27 '10 at 22:08
  • 1
    @dtb: That sounds like it might make an interesting answer if you could demonstrate in code how that would improve on the code I have already. However in many cases I would really prefer to remove the delegate from the event. – AnthonyWJones Jan 27 '10 at 22:09
  • @Josh: I agree it is "spaghetti-ish", so thats another reason to have a tidier solution. – AnthonyWJones Jan 27 '10 at 22:10
  • @josh: The goal is remove the delegate rather than to have it fire and do nothing. – AnthonyWJones Jan 27 '10 at 22:11
  • @Josh, that could result in what amounts to a memory leak if he does it a lot. – Jonathan Allen Jan 27 '10 at 22:16
  • Just asked a similar question today: http://stackoverflow.com/questions/2147116/event-handling-with-an-anonymous-delegate – herzmeister Jan 27 '10 at 22:36
  • This looks like it would fail at detachment for multicast events. – Randolpho Jan 27 '10 at 23:02
  • @Randolpho: Oh? How does a "multicast" event differ from a bog standard event? In what way does it fail? – AnthonyWJones Jan 28 '10 at 08:08
  • @Jonathan Allen - Can you point me to an article that explains more about how the memory leak could occur? I'd really like to learn more. Most of my experience with event handlers has been in Windows Forms and I nearly always create forms within a using block so I figured I didn't have to worry about leaks. – Josh Kodroff Jan 28 '10 at 14:22
  • @AnthonyWJones: A multicast event (any event that returns void), when raised, iterates over a list of subscribed event handlers and calls each event handler in turn -- unless you specifically raise the event asynchronously, which is actually rare. Detaching your event handler modifies that list. Detaching your event handler while it's handling your event modifies that list during iteration of that list. And we all know what happens when you modify a list while iterating over it... – Randolpho Jan 28 '10 at 15:19
  • @Randolpho: ahh, good point, I'll have to look into that. – AnthonyWJones Jan 28 '10 at 16:02
  • @Randolpho: Having looked into this via Reflector and some testing I can't generate any problems using this technique. It would seem that once fired the current set to delegates will all fire regardless of any removals that happen as a result of code running in those delegates. Subsequent event firing results in expected behaviour. To be honest this doesn't surprise me, I'd be more surprised if the .NET hadn't anticipated this scenario and made allowances for it. – AnthonyWJones Jan 29 '10 at 22:23
  • @AnthonyWJones: excellent point. Well, if it works it works. It just looked like it would fail on the surface to me. :) – Randolpho Jan 29 '10 at 22:59

8 Answers8

10

You could attache a permanent event handler to the event. The event handler then invokes "one shot event handlers" that are added to an internal queue:

OneShotHandlerQueue<EventArgs> queue = new OneShotHandlerQueue<EventArgs>();

Test test = new Test();

// attach permanent event handler
test.Done += queue.Handle;

// add a "one shot" event handler
queue.Add((sender, e) => Console.WriteLine(e));
test.Start();

// add another "one shot" event handler
queue.Add((sender, e) => Console.WriteLine(e));
test.Start();

Code:

class OneShotHandlerQueue<TEventArgs> where TEventArgs : EventArgs {
    private ConcurrentQueue<EventHandler<TEventArgs>> queue;
    public OneShotHandlerQueue() {
        this.queue = new ConcurrentQueue<EventHandler<TEventArgs>>();
    }
    public void Handle(object sender, TEventArgs e) {
        EventHandler<TEventArgs> handler;
        if (this.queue.TryDequeue(out handler) && (handler != null))
            handler(sender, e);
    }
    public void Add(EventHandler<TEventArgs> handler) {
        this.queue.Enqueue(handler);
    }
}

Test class:

class Test {
    public event EventHandler Done;
    public void Start() {
        this.OnDone(new EventArgs());
    }
    protected virtual void OnDone(EventArgs e) {
        EventHandler handler = this.Done;
        if (handler != null)
            handler(this, e);
    }
}
dtb
  • 213,145
  • 36
  • 401
  • 431
9

You can use reflection:

public static class Listener {

  public static void ListenOnce(this object eventSource, string eventName, EventHandler handler) {
    var eventInfo = eventSource.GetType().GetEvent(eventName);
    EventHandler internalHandler = null;
    internalHandler = (src, args) => {
      eventInfo.RemoveEventHandler(eventSource, internalHandler);
      handler(src, args);
    };
    eventInfo.AddEventHandler(eventSource, internalHandler);
  }

  public static void ListenOnce<TEventArgs>(this object eventSource, string eventName, EventHandler<TEventArgs> handler) where TEventArgs : EventArgs {
    var eventInfo = eventSource.GetType().GetEvent(eventName);
    EventHandler<TEventArgs> internalHandler = null;
    internalHandler = (src, args) => {
      eventInfo.RemoveEventHandler(eventSource, internalHandler);
      handler(src, args);
    };
    eventInfo.AddEventHandler(eventSource, internalHandler);
  }

}

Use it like so:

variableOfSomeType.ListenOnce("SomeEvent", 
  (s, args) => Console.WriteLine("I should print only once!"));

variableOfSomeType.ListenOnce<InterestingEventArgs>("SomeOtherEvent", 
  (s, args) => Console.WriteLine("I should print only once!"));
Jordão
  • 55,340
  • 13
  • 112
  • 144
  • 1
    real nice. The only thing I would change is in `internalHandler`, the first thing I would call is the `RemoveEventHandler` (what if the `handler` call throws an exception?) – PLopes Nov 22 '18 at 12:13
  • @PLopes: makes a lot of sense, I'll make the change – Jordão Nov 22 '18 at 15:21
8

If you can use the Reactive Extensions for .NET, you can simplify this.

You can make an Observable from an event, and only listen for the first element using .Take(1), to do your small snippet of code. This turns this entire process into a couple of lines of code.


Edit: In order to demonstrate, I've made a full sample program (I'll paste below).

I moved the observable creation and subscription into a method (HandleOneShot). This lets you do what you're attempting with a single method call. For demonstrating, I made a class with two properties that implements INotifyPropertyChanged, and am listening for the first property changed event, writing to the console when it occurs.

This takes your code, and changes it to:

HandleOneShot<SomeEventArgs>(variableOfSomeType, "SomeEvent",  e => { 
                    // Small snippet of code here
                }); 

Notice that all of the subscription/unsubscription happens automatically for you behind the scenes. There's no need to handle putting in the subscription manually - just Subscribe to the Observable, and Rx takes care of this for you.

When run, this code prints:

Setup...
Setting first property...
 **** Prop2 Changed! /new val
Setting second property...
Setting first property again.
Press ENTER to continue...

You only get a single, one shot trigger of your event.

namespace ConsoleApplication1
{
    using System;
    using System.ComponentModel;
    using System.Linq;

    class Test : INotifyPropertyChanged
    {
        private string prop2;
        private string prop;
        public string Prop
        {
            get {
                return prop;
            }
            set
            {
                if (prop != value)
                {
                    prop = value;
                    if (PropertyChanged!=null)
                        PropertyChanged(this, new PropertyChangedEventArgs("Prop"));
                }
            }
        }

        public string Prop2
        {
            get
            {
                return prop2;
            }
            set
            {
                if (prop2 != value)
                {
                    prop2 = value;
                    if (PropertyChanged != null)
                        PropertyChanged(this, new PropertyChangedEventArgs("Prop2"));
                }
            }
        }

        public event PropertyChangedEventHandler PropertyChanged;
    }


    class Program
    {
        static void HandleOneShot<TEventArgs>(object target, string eventName, Action<TEventArgs> action)  where TEventArgs : EventArgs
        {
            var obsEvent = Observable.FromEvent<TEventArgs>(target, eventName).Take(1);
            obsEvent.Subscribe(a => action(a.EventArgs));
        }

        static void Main(string[] args)
        {
            Test test = new Test();

            Console.WriteLine("Setup...");
            HandleOneShot<PropertyChangedEventArgs>(
                test, 
                "PropertyChanged", 
                e =>
                    {
                        Console.WriteLine(" **** {0} Changed! {1}/{2}!", e.PropertyName, test.Prop, test.Prop2);
                    });

            Console.WriteLine("Setting first property...");
            test.Prop2 = "new value";
            Console.WriteLine("Setting second property...");
            test.Prop = "second value";
            Console.WriteLine("Setting first property again...");
            test.Prop2 = "other value";

            Console.WriteLine("Press ENTER to continue...");
            Console.ReadLine();
        }
    }
}
Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • I've not really got into the Reactive Extensions as yet, certainly it would provide the functionality that @dtb seemed to be describing but would still leave a delegate on the event that fires but goes no where? My goal is to remove the unnecessary attachment or do I need to understand Reactive stuff better? – AnthonyWJones Jan 27 '10 at 22:25
  • @AnthonyWJones: It's long, but that's fully working code. You'll need the Rx framework for it to run, and add references to the Rx assemblies, though: http://msdn.microsoft.com/en-us/devlabs/ee794896.aspx – Reed Copsey Jan 27 '10 at 22:56
2

Another user encountered a very similar problem, and I believe the solution in that thread applies here.

In particular, what you have is not an instance of the publish/subscribe pattern, its a message queue. Its easy enough to create your own message queue using a Queue{EventHandler}, where you dequeue events as you invoke them.

So instead of hooking on to an event handler, your "one-shot" events should expose a method allowing clients to add an function to the message queue.

Community
  • 1
  • 1
Juliet
  • 80,494
  • 45
  • 196
  • 228
  • 2
    This assumes, though, that you are writing the class for "variableOfSomeType", and that it's not a framework or third party type. If it is, this doesn't work. – Reed Copsey Jan 27 '10 at 23:16
1

Does it work? If so, then I say go for it. For a one-shot event that looks to be quite elegant.

What I like...

  • If s is garbage collected, so will the event handler.
  • The detaching code is right next to the attaching code, making it easy to see what you are are doing.

You might be able to generalize it, but I'm not entierly sure how to because I can't seem to get a pointer to a event.

Jonathan Allen
  • 68,373
  • 70
  • 259
  • 447
1

Personally, I just create a specialized extension method for whatever type has the event I'm dealing with.

Here's a basic version of something I am using right now:

namespace MyLibrary
{
    public static class FrameworkElementExtensions
    {
        public static void HandleWhenLoaded(this FrameworkElement el, RoutedEventHandler handler)
        {
            RoutedEventHandler wrapperHandler = null;
            wrapperHandler = delegate
            {
                el.Loaded -= wrapperHandler;

                handler(el, null);
            };
            el.Loaded += wrapperHandler;
        }
    }
}

The reason I think this is the best solution is because you often don't need to just handle the event one time. You also often need to check if the event has already passed... For instance, here is another version of the above extension method that uses an attached property to check if the element is already loaded, in which case it just calls the given handler right away:

namespace MyLibraryOrApplication
{
    public static class FrameworkElementExtensions
    {
        public static void HandleWhenLoaded(this FrameworkElement el, RoutedEventHandler handler)
        {
            if ((bool)el.GetValue(View.IsLoadedProperty))
            {
                // el already loaded, call the handler now.
                handler(el, null);
                return;
            }
            // el not loaded yet. Attach a wrapper handler that can be removed upon execution.
            RoutedEventHandler wrapperHandler = null;
            wrapperHandler = delegate
            {
                el.Loaded -= wrapperHandler;
                el.SetValue(View.IsLoadedProperty, true);

                handler(el, null);
            };
            el.Loaded += wrapperHandler;
        }
    }
}
Wayne Bloss
  • 5,370
  • 7
  • 50
  • 81
  • 1
    I have to note that there is a potential race condition here: if you write some similar code that is running in a background-thread, you can miss your event being fired. If your property is loaded between the check for IsLoadedProperty (being false at that point) and the point were you attach the event handler. - You are running this in the UI-thread here, so not a problem with your snippet. I just wanted to point out the pitfall. – toong Mar 05 '13 at 09:02
0

You probably want to work with the new async/await idioms. Usually when I need to execute an event handler one-shot like you described, what I really need is something like:

await variableOfSomeSort.SomeMethodAsync();
//small snippet of code here
Albino Cordeiro
  • 1,554
  • 1
  • 17
  • 25
0

Why not do use the delegate stack built into the event? Something like...

    private void OnCheckedIn(object sender, Session e)
    {
        EventHandler<Session> nextInLine = null; 
        lock (_syncLock)
        {
            if (SessionCheckedIn != null)
            {
                nextInLine = (EventHandler<Session>)SessionCheckedIn.GetInvocationList()[0];
                SessionCheckedIn -= nextInLine;
            }
        }

        if ( nextInLine != null )
        {
            nextInLine(this, e);
        }
    }
  • 1
    Welcome to [so]. I would be advantageous to elaborate on why and how this solves the question. Code should be explained. See [answer]. – jkalden Mar 10 '17 at 08:24