How wrong is it to create an event handler delegate with out the standard (Obj sender, EventArgs args) signature?
Asked Answered
O

3

15

I understand the benefits of using the standard MS event handler delegate signature as it allows you to easily expand on the information passed through the event with out breaking any old relationships that are based on the old delegate signature.

What I'm wondering is in practice how often do people follow this rule? Say I have a simple event like this

public event NameChangedHandler NameChanged;
public delegate void NameChangedHandler(Object sender, string oldName, string newName);

It's a simple event, and I'm nearly positive that the only arguments I'm ever going to need to know from the NameChanged event is the object whose name changed, the old name, and the new name. So is it worth it to create a separate NameChangedEventArgs class, or for simple events like this is it acceptable to just return the the arguments directly through the delegate arguments?

Oxa answered 21/7, 2009 at 3:0 Comment(0)
C
6

You can do anything the wrong way if you're the only one who has to deal with it. But it's not a bad idea to learn standards and stick to them so that you keep good habits when you're working on code with others.

So I'll make you a deal. If you promise to do it the right way, I'll give you a code snippet that'll make it much less of a pain. Just put this in a .snippet file, and put that file in:

My Documents\Visual Studio 2008\Code Snippets\Visual C#\My Code Snippets\
(or Visual Studio 2005 if applicable)

And here's the snippet; use it in VS by typing ev2Generic and hitting Tab:

<?xml version="1.0" encoding="utf-8" ?>
<CodeSnippets  xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
  <CodeSnippet Format="1.0.0">
    <Header>
      <Title>Generic event with two types/arguments.</Title>
      <Shortcut>ev2Generic</Shortcut>
      <Description>Code snippet for event handler and On method</Description>
      <Author>Kyralessa</Author>
      <SnippetTypes>
        <SnippetType>Expansion</SnippetType>
      </SnippetTypes>
    </Header>
    <Snippet>
      <Declarations>
        <Literal>
          <ID>type1</ID>
          <ToolTip>Type of the first property in the EventArgs subclass.</ToolTip>
          <Default>propertyType1</Default>
        </Literal>
        <Literal>
          <ID>arg1Name</ID>
          <ToolTip>Name of the first argument in the EventArgs subclass constructor.</ToolTip>
          <Default>property1Name</Default>
        </Literal>
        <Literal>
          <ID>property1Name</ID>
          <ToolTip>Name of the first property in the EventArgs subclass.</ToolTip>
          <Default>Property1Name</Default>
        </Literal>
        <Literal>
          <ID>type2</ID>
          <ToolTip>Type of the second property in the EventArgs subclass.</ToolTip>
          <Default>propertyType2</Default>
        </Literal>
        <Literal>
          <ID>arg2Name</ID>
          <ToolTip>Name of the second argument in the EventArgs subclass constructor.</ToolTip>
          <Default>property2Name</Default>
        </Literal>
        <Literal>
          <ID>property2Name</ID>
          <ToolTip>Name of the second property in the EventArgs subclass.</ToolTip>
          <Default>Property2Name</Default>
        </Literal>
        <Literal>
          <ID>eventName</ID>
          <ToolTip>Name of the event</ToolTip>
          <Default>NameOfEvent</Default>
        </Literal>
      </Declarations>
      <Code Language="CSharp">
        <![CDATA[public class $eventName$EventArgs : System.EventArgs
      {
        public $eventName$EventArgs($type1$ $arg1Name$, $type2$ $arg2Name$)
        {
          this.$property1Name$ = $arg1Name$;
          this.$property2Name$ = $arg2Name$;
        }

        public $type1$ $property1Name$ { get; private set; }
        public $type2$ $property2Name$ { get; private set; }
      }

      public event EventHandler<$eventName$EventArgs> $eventName$;
            protected virtual void On$eventName$($eventName$EventArgs e)
            {
                var handler = $eventName$;
                if (handler != null)
                    handler(this, e);
            }]]>
      </Code>
    </Snippet>
  </CodeSnippet>
</CodeSnippets>
Consent answered 21/7, 2009 at 3:8 Comment(4)
You can adapt this, of course, to have just one property, or three, or as many as you like. I have snippets for 1, 2, or 3 properties, as well as for the simple case of creating an event with no specific event data.Consent
I am aware this is an old answer, but I would just like to add a link to this thread: Event Signature in .NET — Using a Strong Typed 'Sender'?. IMHO there is no reason to dogmatically follow a convention, if it results in smelly coding practices (casting the sender parameter, for example). The EventArgs class itself also provides no benefit whatsoever, although you could argue that it's better to wrap passed data to maintain source compatibility.Ayers
@Groo, it's an interesting idea. I hadn't seen that thread before. Note that the person who asked the question here was talking about using an event handler delegate with more than two properties, and not using the standard sender, EventArgs pattern. What was at issue here wasn't so much casting sender, as whether or not to use a single EventArgs or EventArgs<T> parameter instead of multiple parameters.Consent
Oh that's right, I didn't read it thoroughly enough. I got the CA1009 design warning and googled a bit to see what the opinions are about it, since I'd like to suppress it, and stumbled upon this.Ayers
C
10

Use the EventHandler<T> generic delegates for your events and create a type derived from EventArgs to hold your event data. So in other words, always. It's something that you always know exactly how it works when you come across it because it's never done otherwise.

Edit:

Code analysis CA1003: Use generic event handler instances
Code analysis CA1009: Declare event handlers correctly

Churchlike answered 21/7, 2009 at 3:4 Comment(2)
Eh, it happens. :) I really hate it when there's no explanation though.Churchlike
@JhonnyD.Cano-Leftware-, the snippet I posted uses EventHandler<T>.Consent
C
6

You can do anything the wrong way if you're the only one who has to deal with it. But it's not a bad idea to learn standards and stick to them so that you keep good habits when you're working on code with others.

So I'll make you a deal. If you promise to do it the right way, I'll give you a code snippet that'll make it much less of a pain. Just put this in a .snippet file, and put that file in:

My Documents\Visual Studio 2008\Code Snippets\Visual C#\My Code Snippets\
(or Visual Studio 2005 if applicable)

And here's the snippet; use it in VS by typing ev2Generic and hitting Tab:

<?xml version="1.0" encoding="utf-8" ?>
<CodeSnippets  xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
  <CodeSnippet Format="1.0.0">
    <Header>
      <Title>Generic event with two types/arguments.</Title>
      <Shortcut>ev2Generic</Shortcut>
      <Description>Code snippet for event handler and On method</Description>
      <Author>Kyralessa</Author>
      <SnippetTypes>
        <SnippetType>Expansion</SnippetType>
      </SnippetTypes>
    </Header>
    <Snippet>
      <Declarations>
        <Literal>
          <ID>type1</ID>
          <ToolTip>Type of the first property in the EventArgs subclass.</ToolTip>
          <Default>propertyType1</Default>
        </Literal>
        <Literal>
          <ID>arg1Name</ID>
          <ToolTip>Name of the first argument in the EventArgs subclass constructor.</ToolTip>
          <Default>property1Name</Default>
        </Literal>
        <Literal>
          <ID>property1Name</ID>
          <ToolTip>Name of the first property in the EventArgs subclass.</ToolTip>
          <Default>Property1Name</Default>
        </Literal>
        <Literal>
          <ID>type2</ID>
          <ToolTip>Type of the second property in the EventArgs subclass.</ToolTip>
          <Default>propertyType2</Default>
        </Literal>
        <Literal>
          <ID>arg2Name</ID>
          <ToolTip>Name of the second argument in the EventArgs subclass constructor.</ToolTip>
          <Default>property2Name</Default>
        </Literal>
        <Literal>
          <ID>property2Name</ID>
          <ToolTip>Name of the second property in the EventArgs subclass.</ToolTip>
          <Default>Property2Name</Default>
        </Literal>
        <Literal>
          <ID>eventName</ID>
          <ToolTip>Name of the event</ToolTip>
          <Default>NameOfEvent</Default>
        </Literal>
      </Declarations>
      <Code Language="CSharp">
        <![CDATA[public class $eventName$EventArgs : System.EventArgs
      {
        public $eventName$EventArgs($type1$ $arg1Name$, $type2$ $arg2Name$)
        {
          this.$property1Name$ = $arg1Name$;
          this.$property2Name$ = $arg2Name$;
        }

        public $type1$ $property1Name$ { get; private set; }
        public $type2$ $property2Name$ { get; private set; }
      }

      public event EventHandler<$eventName$EventArgs> $eventName$;
            protected virtual void On$eventName$($eventName$EventArgs e)
            {
                var handler = $eventName$;
                if (handler != null)
                    handler(this, e);
            }]]>
      </Code>
    </Snippet>
  </CodeSnippet>
</CodeSnippets>
Consent answered 21/7, 2009 at 3:8 Comment(4)
You can adapt this, of course, to have just one property, or three, or as many as you like. I have snippets for 1, 2, or 3 properties, as well as for the simple case of creating an event with no specific event data.Consent
I am aware this is an old answer, but I would just like to add a link to this thread: Event Signature in .NET — Using a Strong Typed 'Sender'?. IMHO there is no reason to dogmatically follow a convention, if it results in smelly coding practices (casting the sender parameter, for example). The EventArgs class itself also provides no benefit whatsoever, although you could argue that it's better to wrap passed data to maintain source compatibility.Ayers
@Groo, it's an interesting idea. I hadn't seen that thread before. Note that the person who asked the question here was talking about using an event handler delegate with more than two properties, and not using the standard sender, EventArgs pattern. What was at issue here wasn't so much casting sender, as whether or not to use a single EventArgs or EventArgs<T> parameter instead of multiple parameters.Consent
Oh that's right, I didn't read it thoroughly enough. I got the CA1009 design warning and googled a bit to see what the opinions are about it, since I'd like to suppress it, and stumbled upon this.Ayers
F
4

In practice how often do people [not use EventArgs derived classes].

I have never encountered a time when EventArgs derived classes have not been used. As you say yourself, it increases your ability to change your code later. I would also argue that the readability is improved, since it's easy to see that this is an event handler.

Is it worth it to create a separate NameChangedEventArgs class, or for simple events like this is it acceptable to just return the arguments directly through the delegate arguments?

You seem to say that you would use EventArgs for event handlers with more params and not use it for instances like this. Honestly, that is simply not an option when programming in C#. Consistency is a must, especially in today's world of forum's like this, open source projects, etc where it's easy to lose it. Sure if you are programming this under a rock, you may do whatever you like, but the greater C# community will thank you for following standards and especially using consistency in you code.

Filtrate answered 21/7, 2009 at 3:15 Comment(1)
Yeah that was really my question. If it a complex event then I understand why you'd want to have flexibility with using EventArgs. But if it's something simple where I can be 99% sure I know all the arguments I'm going to need, I wasn't sure if it really mattered to create a separate EventArgs class with one or two arguments.Oxa

© 2022 - 2024 — McMap. All rights reserved.