Why doesn't this compile?

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

I need to have custom event subscribe and unsubscribe methods in some code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure doesn't.
It doesn't make any sense to me why.
Can anyone tell me what I'm either doing wrong, or what?

class TestEvent
{
public delegate void TestSuccessEventHandler( string message );
public event TestSuccessEventHandler TestSuccess;
public void SendSuccessEvent()
{
if( TestSuccess != null )
TestSuccess( "Test Success" );
}
public delegate void TestFailureEventHandler( string message );
public event TestFailureEventHandler TestFailure
{
add
{
// Do Something Here
TestFailure += value;
}
remove
{
TestFailure -= value;
// Do Something Here
}
}
public void SendFailureEvent()
{
if( TestFailure != null )
TestFailure( "Test Failure" );
}
}

[Jon Skeet [C# MVP]]

Scott S. <Sc****@community.nospamwrote:

I need to have custom event subscribe and unsubscribe methods in some code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure doesn't.
It doesn't make any sense to me why.
Can anyone tell me what I'm either doing wrong, or what?

A "field-like event" (like TestSuccess) declares both an event and a
variable - the variable is what gets referred to within the class, and
the event is what gets referred to outside the class.

Now, you've declared the TestFailure event but no corresponding
variable. Even if your code compiled, you'd get a stack overflow as
soon as you tried to subscribe to the event, because it would just call
the subscriber again. You need a normal variable to actually hold the
delegate instance, and *that's* what the body of add/remove and
SendFailureEvent should refer to.

See http://pobox.com/~skeet/csharp/events.html for more details.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
World class .NET training in the UK: http://iterativetraining.co.uk

[DeveloperX]

On 6 Dec, 13:49, Scott S. <[e-mail]:

I need to have custom event subscribe and unsubscribe methods in some code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure doesn't.
It doesn't make any sense to me why.
Can anyone tell me what I'm either doing wrong, or what?

class TestEvent
{
public delegate void TestSuccessEventHandler( string message );
public event TestSuccessEventHandler TestSuccess;
public void SendSuccessEvent()
{
if( TestSuccess != null )
TestSuccess( "Test Success" );
}

public delegate void TestFailureEventHandler( string message );
public event TestFailureEventHandler TestFailure
{
add
{
// Do Something Here
TestFailure += value;
}
remove
{
TestFailure -= value;
// Do Something Here
}
}
public void SendFailureEvent()
{
if( TestFailure != null )
TestFailure( "Test Failure" );
}
}

Try the code below. I can't really describe why it goes wrong, it's
something to do with what the compiler does with access rights. I'm
sure someone will be able to explain

I added some thread safety features for good measure :)

using System;
namespace WindowsApplication34
{
class TestEvent
{
public delegate void TestSuccessEventHandler( string message );
public event TestSuccessEventHandler TestSuccess;
public void SendSuccessEvent()
{
if( TestSuccess != null )
TestSuccess( "Test Success" );
}
public delegate void TestFailureHandler(object sender, EventArgs
e);

private event TestFailureHandler _testFailure;
private readonly object _eventLock = new object();

public event TestFailureHandler TestFailure
{
add
{
lock(_eventLock)
{
// Do Something Here
_testFailure += value;
}
}
remove
{
lock(_eventLock)
{
_testFailure -= value;
// Do Something Here
}
}
}
protected virtual void OnSendFailure(EventArgs e)
{
//Don't fire if null (which means no one is subscribed.
if (_testFailure != null)
{
//Fire the event
_testFailure(this,e);
}
}
public void SendFailureEvent()
{
OnSendFailure(new EventArgs()); //test
}
}
}

[DeveloperX]

On 6 Dec, 14:14, Jon Skeet [C# MVP] <[e-mail]:

Scott S. <[e-mail]:

I need to have custom event subscribe and unsubscribe methods in some code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure doesn't.
It doesn't make any sense to me why.
Can anyone tell me what I'm either doing wrong, or what?

A "field-like event" (like TestSuccess) declares both an event and a
variable - the variable is what gets referred to within the class, and
the event is what gets referred to outside the class.

Now, you've declared the TestFailure event but no corresponding
variable. Even if your code compiled, you'd get a stack overflow as
soon as you tried to subscribe to the event, because it would just call
the subscriber again. You need a normal variable to actually hold the
delegate instance, and *that's* what the body of add/remove and
SendFailureEvent should refer to.

Seehttp://pobox.com/~skeet/csharp/events.htmlfor more details.

--
Jon Skeet - <[e-mail]>http://www.pobox.com/~skeet Blog:http://www.msmvps.com/jon.skeet
World class .NET training in the UK:http://iterativetraining.co.uk

Good explanation :)

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

I kept looking ... there are very few examples out there using the custome
add/remove "properties" for event.
Most I've seen look just like my code, but I just came across one that
looked a little different and tried that ... it compiled.
The key bit is that declaring your own add and remove "properties" seems to
cause it to not create the usually implicit delegate list variable.
I just finished experimenting with my test code, and it does work.
I'm posting this "correct" usage of add/remove for others edification.

public delegate void TestFailureEventHandler( string message );
private TestFailureEventHandler TestFailureList;
public event TestFailureEventHandler TestFailure
{
add
{
// Do Something Here
TestFailureList += value;
}
remove
{
TestFailureList -= value;
// Do Something Here
}
}
public void SendFailureEvent()
{
if( TestFailureList != null )
TestFailureList( "Test Failure" );
}

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

Thanks Jon!

I _finally_ came across an example that did exactly what you're saying.
So I had found the solution, had just tested it and posted my own reply
before I saw your reply come in.
But thanks again, because I might not have ever found it since most examples
only show the event declaration witht he add and remove methods and many out
there have _exactly_ what I typed in them, which as you pointed out would do
BAD things.

"Jon Skeet [C# MVP]" wrote:

Scott S. <Sc****@community.nospamwrote:

I need to have custom event subscribe and unsubscribe methods in some code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure doesn't.
It doesn't make any sense to me why.
Can anyone tell me what I'm either doing wrong, or what?

A "field-like event" (like TestSuccess) declares both an event and a
variable - the variable is what gets referred to within the class, and
the event is what gets referred to outside the class.

Now, you've declared the TestFailure event but no corresponding
variable. Even if your code compiled, you'd get a stack overflow as
soon as you tried to subscribe to the event, because it would just call
the subscriber again. You need a normal variable to actually hold the
delegate instance, and *that's* what the body of add/remove and
SendFailureEvent should refer to.

See http://pobox.com/~skeet/csharp/events.html for more details.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
World class .NET training in the UK: http://iterativetraining.co.uk

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

I see what you did ... wrapped a public event interface around a private
event allowing the add/remove code to work before allowing the inner event to
function normally. It seems like that would work, but the solution of just
added a delegate list variable and using that is much cleaner.

Also, I like the thread protection ... I may use it inthe future but for now
I don't need it because all my objects are being created in the main thread.
I needed to override the add/remove to do a backround subscription to a
remote message queue so that I didn't have to do that when no objects wanted
events.

Hmmm ... on second thought, when the background object calls
SendSuccessEvent() I wonder if it will be on another thread?
If it is, I wonder what happens if an object tries to subscribe while it is
sending events?
I'm not sure, so I might just put in the thread protection just to be safe!

Thanks for the thought!

"DeveloperX" wrote:

On 6 Dec, 13:49, Scott S. <[e-mail]:

I need to have custom event subscribe and unsubscribe methods in some code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure doesn't.
It doesn't make any sense to me why.
Can anyone tell me what I'm either doing wrong, or what?

class TestEvent
{
public delegate void TestSuccessEventHandler( string message );
public event TestSuccessEventHandler TestSuccess;
public void SendSuccessEvent()
{
if( TestSuccess != null )
TestSuccess( "Test Success" );
}

public delegate void TestFailureEventHandler( string message );
public event TestFailureEventHandler TestFailure
{
add
{
// Do Something Here
TestFailure += value;
}
remove
{
TestFailure -= value;
// Do Something Here
}
}
public void SendFailureEvent()
{
if( TestFailure != null )
TestFailure( "Test Failure" );
}
}

Try the code below. I can't really describe why it goes wrong, it's
something to do with what the compiler does with access rights. I'm
sure someone will be able to explain

I added some thread safety features for good measure :)

using System;
namespace WindowsApplication34
{
class TestEvent
{
public delegate void TestSuccessEventHandler( string message );
public event TestSuccessEventHandler TestSuccess;
public void SendSuccessEvent()
{
if( TestSuccess != null )
TestSuccess( "Test Success" );
}
public delegate void TestFailureHandler(object sender, EventArgs
e);

private event TestFailureHandler _testFailure;
private readonly object _eventLock = new object();

public event TestFailureHandler TestFailure
{
add
{
lock(_eventLock)
{
// Do Something Here
_testFailure += value;
}
}
remove
{
lock(_eventLock)
{
_testFailure -= value;
// Do Something Here
}
}
}
protected virtual void OnSendFailure(EventArgs e)
{
//Don't fire if null (which means no one is subscribed.
if (_testFailure != null)
{
//Fire the event
_testFailure(this,e);
}
}
public void SendFailureEvent()
{
OnSendFailure(new EventArgs()); //test
}
}
}

[DeveloperX]

On 6 Dec, 14:58, Scott S. <[e-mail]:

I see what you did ... wrapped a public event interface around a private
event allowing the add/remove code to work before allowing the inner event to
function normally. It seems like that would work, but the solution of just
added a delegate list variable and using that is much cleaner.

Also, I like the thread protection ... I may use it inthe future but for now
I don't need it because all my objects are being created in the main thread.
I needed to override the add/remove to do a backround subscription to a
remote message queue so that I didn't have to do that when no objects wanted
events.

Hmmm ... on second thought, when the background object calls
SendSuccessEvent() I wonder if it will be on another thread?
If it is, I wonder what happens if an object tries to subscribe while it is
sending events?
I'm not sure, so I might just put in the thread protection just to be safe!

Thanks for the thought!

"DeveloperX" wrote:

On 6 Dec, 13:49, Scott S. <[e-mail]:

I need to have custom event subscribe and unsubscribe methods in some code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure doesn't.
It doesn't make any sense to me why.
Can anyone tell me what I'm either doing wrong, or what?

class TestEvent
{
public delegate void TestSuccessEventHandler( string message );
public event TestSuccessEventHandler TestSuccess;
public void SendSuccessEvent()
{
if( TestSuccess != null )
TestSuccess( "Test Success" );
}

public delegate void TestFailureEventHandler( string message );
public event TestFailureEventHandler TestFailure
{
add
{
// Do Something Here
TestFailure += value;
}
remove
{
TestFailure -= value;
// Do Something Here
}
}
public void SendFailureEvent()
{
if( TestFailure != null )
TestFailure( "Test Failure" );
}
}

Try the code below. I can't really describe why it goes wrong, it's
something to do with what the compiler does with access rights. I'm
sure someone will be able to explain

I added some thread safety features for good measure :)

using System;
namespace WindowsApplication34
{
class TestEvent
{
public delegate void TestSuccessEventHandler( string message );
public event TestSuccessEventHandler TestSuccess;
public void SendSuccessEvent()
{
if( TestSuccess != null )
TestSuccess( "Test Success" );
}

public delegate void TestFailureHandler(object sender, EventArgs
e);

private event TestFailureHandler _testFailure;
private readonly object _eventLock = new object();

public event TestFailureHandler TestFailure
{
add
{
lock(_eventLock)
{
// Do Something Here
_testFailure += value;
}
}
remove
{
lock(_eventLock)
{
_testFailure -= value;
// Do Something Here
}
}
}
protected virtual void OnSendFailure(EventArgs e)
{
//Don't fire if null (which means no one is subscribed.
if (_testFailure != null)
{
//Fire the event
_testFailure(this,e);
}
}
public void SendFailureEvent()
{
OnSendFailure(new EventArgs()); //test
}
}
}- Hide quoted text -

- Show quoted text -

The thread protection is required because delegates are value type. So
if you have two threads trying to add handlers are the same time, the
following can happen:

Thread 1 reads the value of the delegate and sees it currently
contains sub 1.
Thread 2 reads the value of the delegate and sees it currently
contains sub 1.
Thread 1 adds its handler and now the delegate contains sub 1 and
thread 1's sub.
Thread 2 adds its handler and now the delegate contains sub 1 and
thread 2's sub.
The final value of the delegate could now be either sub1 and
thread1sub or sub1 and thread2sub depending on who wrote it back
first!
I don't know if there are any benefits to using a private delegate
over a private event. I've always used an event. I'm sure I had a
reason, but with all this knowledge stuff, after a while I remember
the how but not always the why :) Jon will know for certain *poke*

[Jon Skeet [C# MVP]]

DeveloperX <nn*****@operamail.comwrote:

The thread protection is required because delegates are value type.

No they're not. They're immutable reference types.

So if you have two threads trying to add handlers are the same time, the
following can happen:

Thread 1 reads the value of the delegate and sees it currently
contains sub 1.
Thread 2 reads the value of the delegate and sees it currently
contains sub 1.
Thread 1 adds its handler and now the delegate contains sub 1 and
thread 1's sub.
Thread 2 adds its handler and now the delegate contains sub 1 and
thread 2's sub.

That much is true.

The final value of the delegate could now be either sub1 and
thread1sub or sub1 and thread2sub depending on who wrote it back
first!

Unfortunately, you didn't put enough thread-safety into OnSendFailure:

1) _testFailure can become null between the check and the fire
2) Because you don't take out a lock (or anything similar), there's no
guarantee you'll see the most recent value of the variable

See http://pobox.com/~skeet/csharp/threads/lockchoice.shtml for a
working example.

I don't know if there are any benefits to using a private delegate
over a private event. I've always used an event. I'm sure I had a
reason, but with all this knowledge stuff, after a while I remember
the how but not always the why :) Jon will know for certain *poke*

If it's private, I'd just use a delegate variable - why have both an
event and a variable when the variable is all that gets used anyway?
(There has to be a variable of some kind, otherwise nothing can get
stored.)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
World class .NET training in the UK: http://iterativetraining.co.uk

[DeveloperX]

On 6 Dec, 15:47, Jon Skeet [C# MVP] <[e-mail]:

DeveloperX <[e-mail]:

The thread protection is required because delegates are value type.

No they're not. They're immutable reference types.

So if you have two threads trying to add handlers are the same time, the
following can happen:

Thread 1 reads the value of the delegate and sees it currently
contains sub 1.
Thread 2 reads the value of the delegate and sees it currently
contains sub 1.
Thread 1 adds its handler and now the delegate contains sub 1 and
thread 1's sub.
Thread 2 adds its handler and now the delegate contains sub 1 and
thread 2's sub.

That much is true.

The final value of the delegate could now be either sub1 and
thread1sub or sub1 and thread2sub depending on who wrote it back
first!

Unfortunately, you didn't put enough thread-safety into OnSendFailure:

1) _testFailure can become null between the check and the fire
2) Because you don't take out a lock (or anything similar), there's no
guarantee you'll see the most recent value of the variable

Seehttp://pobox.com/~skeet/csharp/threads/lockchoice.shtmlfor a
working example.

I don't know if there are any benefits to using a private delegate
over a private event. I've always used an event. I'm sure I had a
reason, but with all this knowledge stuff, after a while I remember
the how but not always the why :) Jon will know for certain *poke*

If it's private, I'd just use a delegate variable - why have both an
event and a variable when the variable is all that gets used anyway?
(There has to be a variable of some kind, otherwise nothing can get
stored.)

--
Jon Skeet - <[e-mail]>http://www.pobox.com/~skeet Blog:http://www.msmvps.com/jon.skeet
World class .NET training in the UK:http://iterativetraining.co.uk

Ah yes, very good point. Immutable reference type != value type. I
should of remembered that as I made the same mistake with strings when
I first started :)

I completely forgot to add locking on the raising of the event, my
bad :(

[Peter Duniho]

On Thu, 06 Dec 2007 05:49:01 -0800, Scott S. <Sc****@community.nospam>
wrote:

I need to have custom event subscribe and unsubscribe methods in some
code in
my application when I ran into this problem.
I created this simple class to demostrate the compile problem.
The use of TestSuccess event compiles, but the use of TestFailure
doesn't.

For what it's worth: if you are asking a question and there was some kind
of error, you really should be specific about the error.

Don't write "this compiles, this doesn't". Write the actual complete text
of the compiler error (sure, you could just post the CSxxxx number, but
not all of us have every number memorized :) ).

Same thing goes for exceptions, though that wasn't an issue in this thread.

You did get the information you need, mainly because in this case the
error was so simple and because someone like Jon was reading the code.
But that's not always going to be the case, and you should be in the habit
of posting better questions than the one you posted here. It will go a
long way toward you getting the information you want.

Pete