Why three times get called?

[=?Utf-8?B?VEo=?=]

Hi,

Below is my testing event hadnlder code.

interface IEventTest
{
void Call();
event EventHandler OnEventHandler;
}

public class EventTest : IEventTest {

public EventTest()
{
}

public void Call()
{
if (eventHandler != null)
{
eventHandler(th is, new EventArgs());
}
}

event EventHandler eventHandler;
event EventHandler IEventTest.OnEv entHandler
{
add
{
if (eventHandler != null)
{
lock (eventHandler)
{
eventHandler += value;
}
}
else
{
eventHandler = new EventHandler(va lue);
}
}
remove
{
if (eventHandler != null)
{
lock (eventHandler)
{
eventHandler -= value;
}
}
}
}
}

This is my testing code.

private void button3_Click(o bject sender, EventArgs e)
{
IEventTest et = new EventTest();
et.OnEventHandl er += new EventHandler(et _OnEventHandler );
et.Call();
et.OnEventHandl er -= new EventHandler(et _OnEventHandler );

et.OnEventHandl er += new EventHandler(et _OnEventHandler );
et.Call();
et.OnEventHandl er -= new EventHandler(et _OnEventHandler );

}

void et_OnEventHandl er(object sender, EventArgs e)
{
MessageBox.Show ("Called");
}

I think messagebox should be displayed two times...not three times..
Why the messagebox is showing three times?
As U see the code, I removed the event handler before calling next Call
method.

Thanks,

[Marc Gravell]

Re: Why three times get called?

The problem is here:

eventHandler = new EventHandler(va lue);

When the first one is added, this is essentially a *different* event-
handler. However, there are other problems in the code; locking
"eventHandl er" is incorrect; since we expect this field to be updated,
there is no guarantee that different callers would be locking the same
object. Actually, I very-much doubt that you need to worry about
locking (most code is not expected to be thread-safe).

The simplest approach here would be a field-like event - i.e. all that
code simply becomes:

public event EventHandler OnEventHandler;

If you want an explicit interface implementation, then just a delegate
(note the optional synchronization code here):

private EventHandler eventHandler;
event EventHandler IEventTest.OnEv entHandler
{
//[MethodImpl(Meth odImplOptions.S ynchronized)]
add { eventHandler += value; }
//[MethodImpl(Meth odImplOptions.S ynchronized)]
remove{ eventHandler -= value;}
}

Finally, I'd argue that OnEventHandler is not a good name for an
event; the typical pattern is that the On{...} method is the
equivalent to your Call - i.e.

public event EventHandler Foo;
protected virtual void OnFoo() {
if(Foo!=null) Foo(this,EventA rgs.Empty);
}

Marc