[adn258]

Ok so I always have some trouble with this. I have a program that creates multiple tabs for the user to use and each of them have a RTB in them which I make as the tabpage tag. So that being said depending on which RTB in which tab the user is in I want certain things to change in the program when the user changes the selection location in the rich text boxes in the different tabs. So I create an eventhandler everytime a user selects a tab that they created like so

private void rtb_selection_changed(object sender, EventArgs e)
        {
            Update_Font_Lbl();
        }

        private void tbctrl_main_Selected(object sender, TabControlEventArgs e)
        {
            (this.tbctrl_main.SelectedTab.Tag as RichTextBox).Selectionchanged += new EventHandler(rtb_selection_changed);
        }
   

Someone once told me if an event subscriber lasts longer then the event publisher then you are usually safe. The problem is sometimes it's hard for me to tell what is and isn't the publisher and subscriber. Is this safe code?

[MrShoes]

adn258, on 05 March 2013 - 12:59 AM, said:

Someone once told me if an event subscriber lasts longer then the event publisher then you are usually safe.

Not really true. In fact, bad coding can mean the event subscriber is falsely extending the life of the publisher (for example, if you don't remove event handlers, the GC won't dispose of the publisher because a reference is still being held).

You need some help clarifying publishers and subscribers. In your example, the tbctrl_main control published its Selected event, and you the subscriber (from which you have posted the code, assume it's a form) has responded with the event handler tbctrl_main_Selected. The publisher raises or fires the event, and the subscriber responds. There can be multiple subscribers for an event.

BE VERY CAREFUL. An event is a multicast delegate: that means you can hook up numerous event handler methods to it at once. Here, every time the tbctrl_main is selected, you are adding a new event handler to it. Is that really what you want to do? Are you ever removing it?

This post has been edited by MrShoes: 05 March 2013 - 01:55 AM

[adn258]

MrShoes, on 05 March 2013 - 01:54 AM, said:

adn258, on 05 March 2013 - 12:59 AM, said:

Someone once told me if an event subscriber lasts longer then the event publisher then you are usually safe.

Not really true. In fact, bad coding can mean the event subscriber is falsely extending the life of the publisher (for example, if you don't remove event handlers, the GC won't dispose of the publisher because a reference is still being held).

You need some help clarifying publishers and subscribers. In your example, the tbctrl_main control published its Selected event, and you the subscriber (from which you have posted the code, assume it's a form) has responded with the event handler tbctrl_main_Selected. The publisher raises or fires the event, and the subscriber responds. There can be multiple subscribers for an event.

BE VERY CAREFUL. An event is a multicast delegate: that means you can hook up numerous event handler methods to it at once. Here, every time the tbctrl_main is selected, you are adding a new event handler to it. Is that really what you want to do? Are you ever removing it?

What if I Just did this?

private void tbctrl_main_Selected(object sender, TabControlEventArgs e)
        {
            (this.tbctrl_main.SelectedTab.Tag as RichTextBox).Selectionchanged -= new EventHandler(rtb_selection_changed);
(this.tbctrl_main.SelectedTab.Tag as RichTextBox).Selectionchanged += new EventHandler(rtb_selection_changed);
        }

This would ensure that there's only one event handler for the currently selected Richtextbox in the tabs correct?

[MrShoes]

It's kind of overkill. You're removing and assigning an event handler every time the control is selected. That menas if it's deselected or loses focus it will still have the event handler until selected again, at which point it will be removed and then added again. You should really just add the Selectionchanged event handler in the Form's constructor, then you don't need the Selected event handler.

[CodingSup3rnatur@l-360]

Quote

The problem is sometimes it's hard for me to tell what is and isn't the publisher and subscriber.

This is the general pattern:

publisher.TheEvent += [subscriber].HandlerMethod;

Quote

Not really true. In fact, bad coding can mean the event subscriber is falsely extending the life of the publisher (for example, if you don't remove event handlers, the GC won't dispose of the publisher because a reference is still being held).

I think adn258 had it the right way around. When you subscribe to an event, it is the event publisher that gets a reference to the event subscriber, not the other way around. Therefore, I don't think there is a way for the publisher to be kept alive by the event subscription alone.

[adn258]

CodingSup3rnatur@l-360, on 05 March 2013 - 11:43 AM, said:

Quote

The problem is sometimes it's hard for me to tell what is and isn't the publisher and subscriber.

This is the general pattern:

publisher.TheEvent += [subscriber].HandlerMethod;

Quote

Not really true. In fact, bad coding can mean the event subscriber is falsely extending the life of the publisher (for example, if you don't remove event handlers, the GC won't dispose of the publisher because a reference is still being held).

I think adn258 had it the right way around. When you subscribe to an event, it is the event publisher that gets a reference to the event subscriber, not the other way around. Therefore, I don't think there is a way for the publisher to be kept alive by the event subscription alone.

So you're saying that my code above is safe then? It's not safe though right?

[CodingSup3rnatur@l-360]

It's safe in that the subscription won't keep the form (or whatever object rtb_selection_changed() is defined in) 'alive' longer than necessary (assuming you are not doing anything strange that will extend the 'lives' of the rich text boxes beyond that of the object rtb_selection_changed() is defined in). However, it is probably not correct (or at least, well designed or efficient) code for what you need, due to the reasons MrShoes mentioned in the last part of his first post, and in his second post.

This post has been edited by CodingSup3rnatur@l-360: 06 March 2013 - 01:54 PM