views:

55

answers:

2

I have a object that is a private in my class. If that object fires a event I want to pass the event on to whatever is using my class. Currently I do it this way, I put in my constructor:

cbName.CheckedChanged += ((sender, args) => this.CheckChanged(this,args));

Is there a better way to do this, and are there any gotchas like the class wont be disposed because it has a event to itself and I will need to manually unsubscribe in the dispose function?

Changing the sender from the firing object to this is optional.

Full version of test code

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Drawing;
using System.Data;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace placeholder
{
    internal class FilterBase : UserControl, IFilterObject
    {
        public FilterBase(string name)
        {
            InitializeComponent();
            cbName.CheckedChanged += ((sender, args) => this.CheckChanged(this,args));
            cbName.Name = name;
            this.Name = name;
        }

        private CheckBox cbName;
        /// <summary> 
        /// Required designer variable.
        /// </summary>
        private System.ComponentModel.IContainer components = null;

        /// <summary> 
        /// Clean up any resources being used.
        /// </summary>
        /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
        protected override void Dispose(bool disposing)
        {
            if (disposing && (components != null))
            {
                components.Dispose();
            }
            base.Dispose(disposing);
        }

        #region Component Designer generated code

        /// <summary> 
        /// Required method for Designer support - do not modify 
        /// the contents of this method with the code editor.
        /// </summary>
        private void InitializeComponent()
        {
            this.cbName = new System.Windows.Forms.CheckBox();
            this.SuspendLayout();
            // 
            // cbName
            // 
            this.cbName.AutoSize = true;
            this.cbName.Location = new System.Drawing.Point(4, 4);
            this.cbName.Name = "cbName";
            this.cbName.Size = new System.Drawing.Size(79, 17);
            this.cbName.TabIndex = 0;
            this.cbName.Text = "Filter Name";
            this.cbName.UseVisualStyleBackColor = true;
            // 
            // UserControl1
            // 
            this.AutoScaleDimensions = new System.Drawing.SizeF(6F, 13F);
            this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font;
            this.AutoSize = true;
            this.AutoSizeMode = System.Windows.Forms.AutoSizeMode.GrowAndShrink;
            this.Controls.Add(this.cbName);
            this.Name = "Filter Name";
            this.Size = new System.Drawing.Size(86, 24);
            this.ResumeLayout(false);
            this.PerformLayout();

        }

        #endregion

        public event EventHandler CheckChanged;
        public bool Checked
        {
            get { return cbName.Checked; }
        }
    }
}
+1  A: 

Well, that will prevent the current object from being garbage collected while cbName is alive, but it sounds like that's unlikely to be a problem... and it's inherent in the fact that you've got a reference to this whatever you do (because you want to fire this object's CheckChanged handlers).

One downside of this is that you can't unsubscribe (e.g. in a Dispose method) even if you want to. An alternative is to use an actual method:

private void CbNameCheckedChangedHandler(object sender, EventArgs e)
{
    CheckedChanged(this, args);
}

...
cbName.CheckedChanged += CbNameCheckedChangedHandler;

// If you ever want to remove the handler
cbName.CheckedChanged -= CbNameCheckedChangedHandler;

Note that this is all assuming that your CheckedChanged event is null-safe, e.g. that it's declared as:

public event EventHandler CheckedChanged = delegate {};

Otherwise if no-one has subscribed to the event, you'll get a NullReferenceException when cbName.CheckedChanged fires.

Jon Skeet
Thanks for the Null Reference warning!
Scott Chamberlain
+2  A: 

An event is actually similar to an auto-property. You can define your own add and remove methods that pass the delegate directly to the child control, hence removing an extra level of indirection:

public event EventHandler CheckChanged {
    add { cbName.CheckChanged += value; }
    remove { cbName.CheckChanged -= value; }
}

This will remove an extra Delegate being stored in your class (as a Delegate field is used behind-the-scenes of a standard event)

thecoop
Cool, did not know about that.
Scott Chamberlain
Is there any way I can have my cake and eat it too with changing the sender from cbName to FilterBase using something like this?
Scott Chamberlain
As always, Jon Skeet has a great article on the workings of events here: http://csharpindepth.com/Articles/Chapter2/Events.aspx . Be sure to take a gander about thread-safety if that applies to your situation.
Jesse C. Slicer