CODEWITHSUNDEEP
c#events
Jon Comtois
Jon Comtois

Reputation: 1854

Should my class subscribe to its own public events?

I am using C# 3.0. Following the standard event pattern I have:

    public event EventHandler<EventArgs> SomeEventHappens;

    protected virtual void OnSomeEventHappens(EventArgs e)
    {
        if (SomeEventHappens != null)
        {
            SomeEventHappens(this, e);
        }
    }

    private object _someProperty;

    public object SomeProperty
    {
        get
        {
            return _someProperty;
        }
        private set
        {
            if (_someProperty == value)
            {
                return;
            }
            OnSomeEventHappens(EventArgs.Empty);
            _someProperty = value;
        }
    }

Within my same class I would like to take some actions when SomeProperty changes. The way I see it I have 3 alternatives:

1) Do stuff within my SomeProperty setter. Something rubs me the wrong way about doing this since I try to subscribe to the philosophy of everything should do one thing and do it well. Cramming stuff into a setter seems to go against that, or at least has the propensity to.

2) Do stuff in OnSomeEventHappens. Again, seems to go a little against keeping this in simple pieces. Also, if this method gets overridden, could potentially lose functionality if the implementer does not call the base method.

3) Have the class subscribe to SomeEventHappens. To me this seems to be the proper choice as far as encapsulation is concerned, and seems pretty clean. Again, possible repercussions if OnSomeEventHappens is overridden.

Maybe there is something more elegant? I cannot decide between option 2 and 3, and I am curious as to what the Best Practice is. Maybe the safest place is in the property setter after all.

Thoughts?

Update: Thanks for the great comments and answers below. I have learned that it is "OK" to have a class subscribe to its own events, although in my case I am leaning to not do due to overhead. I have put thought into the behavior of potential overriders of my virtual methods and what exactly I want to happen.

In my real-world case, I do not really want the events to be raised without the property being set. As the answers below have guided my thought process, I think I may go with option 1, due to the lower overhead, the reduced risk of improper behavior from inheritors, and it just generally makes better sense to me. Thanks again!

Upvotes: 10

Views: 2642

Answers (4)

Marc
Marc

Reputation: 9354

If you own the state of your own object, catching events just sounds wrong to me. I would go with a separate virtual method. Don't interfere with your event and hope the children throw it. Maybe this would look something like:

    private object _someProperty;
    public object SomeProperty
    {
        get
        {
            return _someProperty;
        }
        private set
        {
            if (_someProperty != value)
            {
              OnSettingSomeProperty(_someProperty, value);
              OnSomeEventHappens(EventArgs.Empty);
              _someProperty = value;
            }
        }
    }

    protected virtual void OnSettingSomeProperty(object oldValue, object newValue)
    {
        // children can play here, validate and throw, etc.
    }

Upvotes: 1

dthorpe
dthorpe

Reputation: 36082

In object frameworks outside of .NET, an object that subscribes to its own events is frowned upon primarily because such things lead to circular references that could keep the object alive indefinitely. This is not an issue in .NET, but it still seems "strange" to me for a object to grope itself this way.

If a class always needs to be aware of when changes happen to the property, your best bet IMO is to make the OnSomeEventHappens method virtual and override it in descendant classes that need the additional info. It's ok to put code in the event firing method. The event firing method is there precisely so that everyone who wants to fire that event has a uniform way to do it.

If you only occasionally need to be informed of when the property changes, then I suppose subscribing and unsubscribing to the event would be appropriate.

Upvotes: 2

BlueMonkMN
BlueMonkMN

Reputation: 25601

If you call SomeEventHappens and OnSomeEventHappens from some common location (the property procedure or another function), then you don't have to worry about overriders neglecting to raise the event. I would prefer to override a function rather than handle events because there's less overhead.

Upvotes: 2

Jon Skeet
Jon Skeet

Reputation: 1500225

Will you always want to take this action, or might you want to subscribe and unsubscribe? In the latter case, option 3 is clearly a good idea.

Is the action you wish to take the kind of action that another class may wish to take? Again, that would lean towards option 3.

Is the action you wish to take inherently part of setting the property? If so, action 1 may be advisable.

Option 3 does sound like a nice "light touch" approach to me.

Upvotes: 1

Related Questions