Firing events from a constructor.

[damiensawyer]

Hi,

I am creating a class (from a base class) that needs to trigger events
(code at bottom). I am instatiating the classes and wiring up the
events as follows.

clsDetermineConnection oDC = new
clsDetermineConnection(Request);
oDC.LogMessage += new
RunTraceBase.TraceArguments(LogMessagesFromEvents);

The issue is that, because the event is not wired up to
LogMessagesFromEvents until after the constructor fires, constructor
events aren't trapped. Is there any nice workaround to this that I'm
missing? Other of course than calling an 'initialise' method after
instatiation?

Thanks very much in advance,


Damien




/// <summary>
/// Abstract class to implement logging events on custom classes.
/// </summary>
abstract protected class RunTraceBase
{
public delegate void TraceArguments(object Sender,
TraceEventArgs args);
public event TraceArguments LogMessage;
protected void RaiseLogMessage(string Message)
{
if (LogMessage != null) LogMessage(this, new
TraceEventArgs(Message));
}

// custom attributes
public class TraceEventArgs : System.EventArgs
{
private string message;
public TraceEventArgs(string m)
{
this.message = m;
}
public string Message()
{
return message;
}
}
}

protected class clsDetermineConnection : RunTraceBase
{.........

/// <summary>
/// Constructor
/// </summary>
/// <param name="Request"></param>
public clsDetermineConnection(HttpRequest Request)
{
RaiseLogMessage("Request Host=blah blah blah"); // Note -
won't fire on the constructor if the event hasn't been wired up yet.
}

 

[Marc Gravell]

This is not thread-safe.

Just a minor point; most classes don't *need* to be thread-safe (and
indeed, aren't), so in most cases the local variable step is overkill.
This would only be an issue in a small handful of cases.

Also - one of Jon's tricks (that I tend not to use personally, as I'd
usually prefer to deal with the null than have an unnecessary heap
object per instance):

using System;

class Foo
{
public event EventHandler Bar = delegate { };

public void OnBar() // normally protected virtual...
{
Bar(this, EventArgs.Empty);
}

static void Main()
{
Foo foo = new Foo();
foo.Bar += delegate { Console.WriteLine("Hi"); };
foo.OnBar();
}
}

Finally - a bit random and maybe off-topic, but I was *going* to say
that if you have a number of events, you might throw in a helper
method like:
protected void OnEvent(EventHandler handler)
{
if (handler != null) handler(this, EventArgs.Empty);
}
to do this for you, so you can just write OnEvent(Bar); - but if you
have multiple events, then EventHandlerList is a better option since
if we assume that only a portion of events are hooked per instance, we
are making quite a saving by treating it as sparse - for example
(showing only 1 event) see below.

Marc

private EventHandlerList events;
protected void ClearEvents() { events = null; }
protected void AddHandler(object key, EventHandler handler)
{
if (key == null) throw new ArgumentNullException("key");
if (handler != null)
{
if (events == null) events = new EventHandlerList();
events.AddHandler(key, handler);
}
}
protected void RemoveHandler(object key, EventHandler handler)
{
if (key == null) throw new ArgumentNullException("key");
if (events != null && handler != null)
{
events.RemoveHandler(key, handler);
}
}
protected void OnEvent(object key)
{
if (key == null) throw new ArgumentNullException("key");
if (events != null)
{
EventHandler handler = (EventHandler)events[key];
if (handler != null) handler(this, EventArgs.Empty);
}
}

// actual events
private static readonly object BarKey = new object();
public event EventHandler Bar
{
add { AddHandler(BarKey, value); }
remove { RemoveHandler(BarKey, value); }
}
public void DoSomet()
{
OnEvent(BarKey);
}

 

[Marc Gravell]

Yes, that is what Component (and thus Control) use to provide sparse
events; re the linear search, dictionary would only start to be
quicker for high numbers of events - for most typical classes (i.e.
<100 events), linear search is probably still the faster option.
Actually, the main gripe I have with EventHandlerList is that I can't
check whether it is empty. It would be nice to throw away the list if
the last handler unsubscribes...

Marc

 

[damiensawyer]

Thanks to both of you - that was great!

Lots to play with there - however chances are I'll implement most of
it.

For the record, with regards to my original question, I ended up
taking the option of passing in logging function using a delegate.

Cheers,


Damien

 

[Marc Gravell]

Damien; glad to help ;-p

Pete - did a quick test, and for me the "get" time balances
(dictionary vs eventhandlerlist) at around 150 (subscribed) events,
based on still using an object key; dictionary has the higher
insertion cost, of course. I've taken into account the need to do a
Delegate.Combine to be comparable to EventHandlerList.

Marc

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
static class Program
{

static void Main()
{
RunTest(1,1); // JIT

RunTest(10, 5000); // JIT
RunTest(50, 5000); // JIT
RunTest(100, 5000); // JIT
RunTest(150, 5000); // JIT
RunTest(200, 5000); // JIT
}
static void RunTest(int count, int repeat)
{
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
object[] keys = new object[count];
EventHandler[] handlers = new EventHandler[count];
for (int i = 0; i < keys.Length; i++)
{
keys = new object();
handlers = delegate { };
}

Stopwatch watch = Stopwatch.StartNew();
EventHandlerList list = null;
for (int r = 0; r < repeat; r++)
{
list = new EventHandlerList();
for (int i = 0; i < keys.Length; i++)
{
list.AddHandler(keys, handlers);
}
for (int i = 0; i < keys.Length; i++)
{
list.AddHandler(keys, handlers);
}
}
watch.Stop();
Console.WriteLine("EventHandlerList:Add 2x{0}x{1}; {2}",
count,repeat,watch.ElapsedMilliseconds);

watch.Reset();
watch.Start();
for (int r = 0; r < repeat; r++)
{
for (int i = 0; i < keys.Length; i++)
{
EventHandler handler = (EventHandler)list[keys];
}
}
watch.Stop();
Console.WriteLine("EventHandlerList:Get {0}x{1}; {2}",
count, repeat, watch.ElapsedMilliseconds);

watch.Reset();
watch.Start();
Dictionary<object, EventHandler> dict = null;
for (int r = 0; r < repeat; r++)
{
dict = new Dictionary<object, EventHandler>();
for (int i = 0; i < keys.Length; i++)
{
EventHandler old;
if (dict.TryGetValue(keys, out old))
{
dict[keys] =
(EventHandler)Delegate.Combine(old, handlers);
}
else
{
dict.Add(keys, handlers);
}
}
for (int i = 0; i < keys.Length; i++)
{
EventHandler old;
if (dict.TryGetValue(keys, out old))
{
dict[keys] =
(EventHandler)Delegate.Combine(old, handlers);
}
else
{
dict.Add(keys, handlers);
}
}
}
watch.Stop();
Console.WriteLine("Dictionary:Add 2x{0}x{1}; {2}",
count, repeat, watch.ElapsedMilliseconds);

watch.Reset();
watch.Start();
for (int r = 0; r < repeat; r++)
{
for (int i = 0; i < keys.Length; i++)
{
EventHandler handler = dict[keys];
}
}
watch.Stop();
Console.WriteLine("Dictionary:Get {0}x{1}; {2}",
count, repeat, watch.ElapsedMilliseconds);


}
}

 

[Chris Shepherd]

Also - one of Jon's tricks (that I tend not to use personally, as I'd
usually prefer to deal with the null than have an unnecessary heap
object per instance):

You could just have one static empty delegate that takes care of default
event subscription. I've used this before in some of my homebrew apps:
ie (simplifying):
public static EventHandler DefaultHandler = delegate { };
public event EventHandler myEvent = SomeClass.DefaultHandler;

If we had extension properties we could even add our own
EventHandler.Default or something along those lines.

It would be nice IMO if the compiler itself did that automatically for
you. You always want to be able to just say "invoke this event"
regardless of whether or not anyone is listening.

Chris.

 

[Marc Gravell]

If we had extension properties...

Well, if we bring extension methods into the party we can deal with
them at the caller:

public static class EventExt
{
public static void SafeInvoke(this EventHandler handler, object
sender)
{
if (handler != null) handler(sender, EventArgs.Empty);
}
public static void SafeInvoke<TEventArgs>(this
EventHandler<TEventArgs> handler,
object sender, TEventArgs args) where TEventArgs : EventArgs
{
if (handler != null) handler(sender, args);
}
}

But you'd need to keep adding extension methods for the other event-
types...

Marc

 

[Jon Skeet [C# MVP]]

You could just have one static empty delegate that takes care of default
event subscription. I've used this before in some of my homebrew apps:
ie (simplifying):
public static EventHandler DefaultHandler = delegate { };
public event EventHandler myEvent = SomeClass.DefaultHandler;

Yes - the only problem with this is that each type of event handler
needs its own field. You could do it for a generic
EventHandler<TEventArgs> though. Using "delegate {}" directly just
means I can always be absolutely consistent for any delegate type
which doesn't have to return a value.

If we had extension properties we could even add our own
EventHandler.Default or something along those lines.

It would be nice IMO if the compiler itself did that automatically for
you. You always want to be able to just say "invoke this event"
regardless of whether or not anyone is listening.

Yes. I can see why it doesn't, but it would be nice to have some way
of asking it to do so in common cases - or have a general "build a no-
op handler for this if one doesn't already exist" library call.

Jon

 

[Chris Shepherd]

Yes - the only problem with this is that each type of event handler
needs its own field. You could do it for a generic
EventHandler<TEventArgs> though. Using "delegate {}" directly just
means I can always be absolutely consistent for any delegate type
which doesn't have to return a value.

True. Come to think of it most of the events I've dealt with are UI
events, which are generally just the EventHandler type.
I wasn't criticizing btw, just offering up a discussion point.

Yes. I can see why it doesn't, but it would be nice to have some way
of asking it to do so in common cases - or have a general "build a no-
op handler for this if one doesn't already exist" library call.

Even an attribute to decorate the event would be an acceptable
compromise for me.

[AutoGenerateEventHandlerDelegate]
public event EventHandler myEvent;

Or possibly consider doing the reverse -- suppressing the
auto-generation in cases where it makes sense.


Chris.

 

[Chris Shepherd]

If we had extension properties...

Well, if we bring extension methods into the party we can deal with
them at the caller: [...]
But you'd need to keep adding extension methods for the other event-
types...

Good point. It looks like any way conceivable would require modification
to the compiler and/or a new language feature.

Chris.

 

[Jon Skeet [C# MVP]]

True. Come to think of it most of the events I've dealt with are UI
events, which are generally just the EventHandler type.
I wasn't criticizing btw, just offering up a discussion point.

Ditto. If I ever start complaining because people bring up
alternatives to my own style, please take away my keyboard.

Yes. I can see why it doesn't, but it would be nice to have some wa
of asking it to do so in common cases - or have a general "build a no-
op handler for this if one doesn't already exist" library call.

Even an attribute to decorate the event would be an acceptable
compromise for me.

[AutoGenerateEventHandlerDelegate]
public event EventHandler myEvent;

Possibly. There would be some oddities around though - imagine an
event in a struct. (Admittedly that's clearly something to avoid to
start with...)

Or possibly consider doing the reverse -- suppressing the
auto-generation in cases where it makes sense.

I think I'd want to work out all the edge cases first, but it's an
interesting idea.

Jon

 

[Chris Shepherd]

Possibly. There would be some oddities around though - imagine an
event in a struct. (Admittedly that's clearly something to avoid to
start with...)

Hmm. How do events on structs even work? Does it just copy the entire
list of subscribed events each time, or does it somehow go deeper than that?

Structs could simply have the caveat that they can't be auto-subscribed
to a default delegate.

I think I'd want to work out all the edge cases first, but it's an
interesting idea.

Once I get my subscription details setup at work again, I'll have to
peruse the Connect site and see if there's any existing suggestions
along these lines.

Chris.

 

[Jon Skeet [C# MVP]]

Hmm. How do events on structs even work? Does it just copy the entire
list of subscribed events each time, or does it somehow go deeper than that?

It would just copy the field, which is a reference to a delegate. The
delegate object itself contains the list.

Structs could simply have the caveat that they can't be auto-subscribed
to a default delegate.
Yup.



Once I get my subscription details setup at work again, I'll have to
peruse the Connect site and see if there's any existing suggestions
along these lines.

I had one along the lines of having a method or language construct to
return a "no-op" delegate. Was closed as "won't fix" which I didn't
mind given the delegate{} kludge.

Jon