CODEWITHSUNDEEP
c#.netevents
Calvin
Calvin

Reputation: 710

CA1009: Declare event handlers correctly?

I have the following event that consumers of my class can wire up with to get internal diagnostic messages.

public event EventHandler<string> OutputRaised;

I raise the event with this function

protected virtual void OnWriteText(string e)
    {
        var handle = this.OutputRaised;
        if (handle != null)
        {
            var message = string.Format("({0}) : {1}", this.Port, e);
            handle(this, message);
        }
    }

Why am I getting CA1009 Declare event handlers correctly? All the answers I found don't seem really applicable to my scenario... Just trying to understand, I don't have a real solid grasp of events and delegates yet.

Reference on CA1009: http://msdn.microsoft.com/en-us/library/ms182133.aspx

Upvotes: 11

Views: 8299

Answers (2)

Frederik Gheysels
Frederik Gheysels

Reputation: 56934

According to 'the rules', the type-parameter of EventHandler should inherit from EventArgs:

Event handler methods take two parameters. The first is of type System.Object and is named 'sender'. This is the object that raised the event. The second parameter is of type System.EventArgs and is named 'e'. This is the data that is associated with the event. For example, if the event is raised whenever a file is opened, the event data typically contains the name of the file.

In your case, that could be something like this:

public class StringEventArgs : EventArgs
{
   public string Message {get;private set;}

   public StringEventArgs (string message)
   {
      this.Message = message;
   }

}

and your eventhandler:

public event EventHandler<StringEventArgs> OutputRaised;

When you raise the event, you should offcourse create an instance of the StringEventArgs class:

protected virtual void OnWriteText( string message )
{
    var handle = this.OutputRaised;
    if (handle != null)
    {
        var message = string.Format("({0}) : {1}", this.Port, e);
        handle(this, new StringEventArgs(message));
    }
}

I also would like to add, that theoretically, there's nothing wrong with your code. The compiler doesn't complain and your code will work. The EventHandler<T> delegate does not specify that the type parameter should inherit from EventArgs. It's FxCop that signals that you're violating the 'design rules' for declaring an event.

Upvotes: 15

Trevor Pilley
Trevor Pilley

Reputation: 16393

Events in .NET should usually contain some derivative of EventArgs which yours does not so I'd guess that's the problem.

Define the event args to be published by the event:

public class StringEventArgs : EventArgs
{
    public StringEventArgs(string message) { this.Message = message; }
    public string Message { get; private set; }
}

Change your Event declaration and publish method:

public event EventHandler<StringEventArgs> OutputRaised;

protected virtual void OnWriteText(string e)
{
    var handle = this.OutputRaised;
    if (handle != null)
    {
        var message = string.Format("({0}) : {1}", this.Port, e);
        handle(this, new StringEventArgs(message));
    }
}

Upvotes: 3

Related Questions