views:

436

answers:

5

There is a property, it's named ImageFullPath1

public string ImageFullPath1 {get; set; }

I'm gonna fire an event whenever its value changed. I know I can beware of changing with INotifyPropertyChanged, but I wanna do it with events.
I don't know how I should do it.
Could you please guide me?

Thanks.

+3  A: 
public event EventHandler ImageFullPath1Changed;

public string ImageFullPath1
{
    get
    {
        // insert getter logic
    }
    set
    {
        // insert setter logic       

        // EDIT -- this example is not thread safe -- do not use in production code
        if (ImageFullPath1Changed != null && value != _backingField)
            ImageFullPath1Changed(this, new EventArgs(/*whatever*/);
    }
}                        

That said, I completely agree with Ryan. This scenario is precisely why INotifyPropertyChanged exists.

Richard Berg
This event invocation has a race condition. The value of `ImageFullPath1Changed` can change to `null` between the check and the subsequent invocation. Do not invoke events like this!
Aaronaught
Your check for null of the ImageFullPath1Changed event is not safe. Given that events can be subscribed/unsubscribed to/from asynchronously from outside of your class, it could become null after your null check and cause a NullReferenceException. Instead, you should take a local copy before checking for null. See Aaronaught's answer.
Simon P Stevens
In the STA model used by WPF and Silverlight, almost everything happens on the UI thread. Regardless, I agree that sample code -- even sample code banged out in 30 seconds on a nonauthoritative website -- should be held to a high standard lest people copy/paste it into their apps without thinking. Thanks.
Richard Berg
Edited to clarify.
Richard Berg
+2  A: 

If you change your property to use a backing field (instead of an automatic property), you can do the following:

public event EventHandler ImageFullPath1Changed;
private string _imageFullPath1 = string.Empty;

public string ImageFullPath1 
{
  get
  {
    return imageFullPath1 ;
  }
  set
  {
    if (_imageFullPath1 != value)
    { 
      _imageFullPath1 = value;

      EventHandler handler = ImageFullPathChanged;
      if (handler != null)
        handler(this, e);
    }
  }
}
Oded
Your check for null of the ImageFullPath1Changed event is not safe. Given that events can be subscribed/unsubscribed to/from asynchronously from outside of your class, it could become null after your null check and cause a NullReferenceException. Instead, you should take a local copy before checking for null. See Aaronaught's answer
Simon P Stevens
@Simon P Stevens - thanks for the info. Updated answer to reflect.
Oded
@Oded. Nice one. Downvote removed.
Simon P Stevens
+4  A: 

Raising an event when a property changes is precisely what INotifyPropertyChanged does. There's one required member to implement INotifyPropertyChanged and that is the PropertyChanged event. Anything you implemented yourself would probably be identical to that implementation, so there's no advantage to not using it.

Ryan Brunner
+1 for truth. Even if you want to implement a separate `XChangedEvent` for every property, you're already doing the work, so go ahead and implement INotifyPropertyChanged too. The future (like WPF) will thank you, because that's what the future expects you to do.
Greg D
+9  A: 

The INotifyPropertyChanged interface is implemented with events. The interface has just one member, PropertyChanged, which is an event that consumers can subscribe to.

The version that Richard posted is not safe. Here is how to safely implement this interface:

public class MyClass : INotifyPropertyChanged
{
    private string imageFullPath;

    protected void OnPropertyChanged(PropertyChangedEventArgs e)
    {
        PropertyChangedEventHandler handler = PropertyChanged;
        if (handler != null)
            handler(this, e);
    }

    protected void OnPropertyChanged(string propertyName)
    {
        OnPropertyChanged(new PropertyChangedEventArgs(propertyName));
    }

    public string ImageFullPath
    {
        get { return imageFullPath; }
        set
        {
            if (value != imageFullPath)
            {
                imageFullPath = value;
                OnPropertyChanged("ImageFullPath");
            }
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;
}

Note that this does the following things:

  • Abstracts the property-change notification methods so you can easily apply this to other properties;

  • Makes a copy of the PropertyChanged delegate before attempting to invoke it (failing to do this will create a race condition).

  • Correctly implements the INotifyPropertyChanged interface.

If you want to additionally create a notification for a specific property being changed, you can add the following code:

protected void OnImageFullPathChanged(EventArgs e)
{
    EventHandler handler = ImageFullPathChanged;
    if (handler != null)
        handler(this, e);
}

public event EventHandler ImageFullPathChanged;

Then add the line OnImageFullPathChanged(EventArgs.Empty) after the line OnPropertyChanged("ImageFullPath").

Aaronaught
+1 for being the only person in this thread so far to get the null check on the event correct.
Simon P Stevens
+2  A: 

I use largely the same patterns as Aaronaught, but if you have a lot of properties it could be nice to use a little generic method magic to make your code a little more DRY

public class TheClass : INotifyPropertyChanged {
    private int _property1;
    private string _property2;
    private double _property3;

    protected virtual void OnPropertyChanged(PropertyChangedEventArgs e) {
        PropertyChangedEventHandler handler = PropertyChanged;
        if(handler != null) {
            handler(this, e);
        }
    }

    protected void SetPropertyField<T>(string propertyName, ref T field, T newValue) {
        if(!EqualityComparer<T>.Default.Equals(field, newValue)) {
            field = newValue;
            OnPropertyChanged(new PropertyChangedEventArgs(propertyName));
        }
    }

    public int Property1 {
        get { return _property1; }
        set { SetPropertyField("Property1", ref _property1, value); }
    }
    public string Property2 {
        get { return _property2; }
        set { SetPropertyField("Property2", ref _property2, value); }
    }
    public double Property3 {
        get { return _property3; }
        set { SetPropertyField("Property3", ref _property3, value); }
    }

    #region INotifyPropertyChanged Members

    public event PropertyChangedEventHandler PropertyChanged;

    #endregion
}

Usually i also make the OnPropertyChanged method virtual to allow sub-classes to override it to catch property changes.

Mikael Sundberg