Reputation: 1472
I have a DataGrid
control in which the user can select multiple items. After selecting the items and pressing a button, I iterate over each and edit a property. When this property is changed, the INotifyPropertyChanged
events fire. It all works beautifully. However I have noticed that if I select two or more items and scroll such that one of the items is well out of view and then press the button the program will crash and give me a null reference exception in the NotifyPropertyChanged()
method telling me that the PropertyChangedEventHandler
is null. The code works normally if I remove the NotifyPropertyChanged()
call from the property's setter, so for now that's what I'm doing, but why would this happen? It works as expected regardless of scroll position if I only have one item selected.
In my model class, which implements the INotifyPropertyChanged
interface, I have these:
public event PropertyChangedEventHandler PropertyChanged;
// ...
public bool IsScheduled
{
get { return isScheduled; }
set { isScheduled = value; NotifyPropertyChanged(self.ToString()); }
}
// ...
private void NotifyPropertyChanged(string propertyName)
{
if (PropertyChanged != null)
PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
}
Elsewhere in the ViewModel I make this simple call which triggers the error:
process.IsScheduled = true;
If I check the values of the selected values list when the error occurs, all of the selected items are detected. it seems something happens with the event when they are invisible.
Why does the visibility of the item in the view affect whether the event handler is null?
Upvotes: 1
Views: 2557
Reputation: 405
simpler even still:
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(caller));
vs
if(PropertyChanged != null)
PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
1 line vs 2
Upvotes: 0
Reputation: 8404
Change your code like so:
public event PropertyChangedEventHandler PropertyChanged;
protected virtual void NotifyPropertyChanged(string propertyName)
{
var handler = PropertyChanged;
if (handler != null)
handler(this, new PropertyChangedEventArgs(propertyName));
}
Always check for null
because if there is no event handlers attached, it will throw NullReferenceException
. Besides that you should always use temp variable when raising events because any object can add/remove handlers to it at any time.
Edit:
Depending on your .NET Framework version, you could also improve the INotifyPropertyChanged
implementation
public event PropertyChangedEventHandler PropertyChanged;
protected virtual void NotifyPropertyChanged([CallerMemberName] string propertyName = "" )
{
var handler = PropertyChanged;
if (handler != null)
handler(this, new PropertyChangedEventArgs(propertyName));
}
And then notify simply using NotifyPropertyChanged();
without explicitly passing property name to method since [CallerMemberName]
gives you the the name of the caller automagically.
This (might) lead to less typing and less room for bugs in your code due to misspelled or changed property names.
Upvotes: 3
Reputation: 3451
Best practise with any event handler is to test for null before calling it...
if(PropertyChanged != null) this.PropertyChanged....
I'm also curious why you would let a string.empty default value be used?
Because the handler is created within WPF and not explicitly created in code, you have no control about whether a handler exists - add the check and you should be ok.
Upvotes: 1