Anything wrong with short-lived event handlers?

For example, is there anything wrong with something like this?

private void button1_Click(object sender, EventArgs e)
{
    Packet packet = new Packet();

    packet.DataNot开发者_JAVA百科Sent += packet_DataNotSent;

    packet.Send();
}

private void packet_DataNotSent(object sender, EventArgs e)
{
    Console.WriteLine("Packet wasn't sent.");
}

class Packet
{
    public event EventHandler DataNotSent;

    public void Send()
    {
        if (DataNotSent != null)
        {
            DataNotSent(null, EventArgs.Empty);
        }
    }
}

If it were just a simple integer variable declaration it would be fine because once it went out of scope and would later be collected by the garbage collector but events are a bit .. longer-living? Do I have to manually unsubscribe or take any sort of measures to make sure this works properly?

It just feels.. weird. Not sure if it is correct or not. Usually when I add to an event handler it lasts for the entirety of the application so I'm not sure about what happens when constantly adding and removing event handlers.

---

Technically, there is nothing wrong with this code. Since there won't be any references to packet after the method finishes, the subscription is going to be (eventually) collected too.

As a matter of style, doing this is very unusual, I think. Better solution would probably be to return some kind of result from Send() and base your next action on that. Sequential code is easier to understand.

---

That's really the beauty of event-based programming -- you don't really care who / what / if anyone is listening, you just say something happened and let whoever subscribed react accordingly.

You don't have to manually unsubscribe but you should unsubscribe as the object goes out scope. The link will keep the subscribers alive (from @pst).

---

I understand that you can get a memory leak if the event source i.e. Packet class and the event sink i.e. the handler of the event have a lifetime mismatch.

In this case the delegate that you create is scoped to this function because the event sink is only scoped to this function.

You can make a call to packet.DataNotSent -= packet_DataNotSent; after the send method executes to ensure that the delegate is garbage collected. Please read this

---

I guess you've simplified the example for brevity, but in your snippet, there is no need to use an event. Packet.Send() could just return a result that is checked. You only really need a more complicated approach if there is going to be some asynchronicity (e.g. an asynchronous operation, scheduling for future execution, etc.), and Packet.Send() does not return immediately.

In terms of object life-cycle management, your event subscription does not pose a problem, since an event subscription will keep the handler alive but not vice versa. That is, the class that button1_Click belongs to will not be garbage collected while there is a live reference to a packet that it has subscribed to. Since the packets have a short lifespan, this won't be a problem.

If in your real-world usage, Packet.Send() cannot return the result, then I would be tempted to pass a delegate to the packet rather than subscribe to an event on it, assuming only one object needs to be notified of the failure.

---

When the object that publishes the event becomes eligible for garbage collection, all the objects subscribed to that event become eligible for garbage collection as well (provided there are no other references to them).

So, in your example:

• When packet goes out of scope, it becomes eligible for GC.
• So the object containing packet_DataNotSent become eligible for GC as well (unless referenced by something else).
• If the object containing packet_DataNotSent is referenced by something else, it will of course not be GC-ed, but it will still automatically "unsibscribe" when packet is GC-ed.

---

Events should be used for scenarios where one doesn't know who might be interested in being notified when something happens or some action becomes necessary. In your example, if anyone's interested in knowing when the packet is discovered to be undeliverable, the code calling the constructor is apt to know who that would be. It would thus be better to have the packet's constructor accept an Action<Packet, PacketResult> delegate than publish an event for that purpose.