Does the NotifyPropertyChanged event ever fire? And: why not?

Sep 1, 2014 at 3:25 PM
The project: a small notice board with "post-it" messages. VB.Net, VS2013, Windows 8.1 Desktop. Prism / MVVM.

The question: the NotifyPropertyChanged event seems not to fire at all when changing properties.

The code:

First, we have the View. It is defined like so:
<prism:VisualStateAwarePage 
    x:Name="pageRoot"
    x:Class="MemoBoard.Views.BoardPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:prism="using:Microsoft.Practices.Prism.StoreApps"
    prism:ViewModelLocator.AutoWireViewModel="True"
    mc:Ignorable="d">
...
Furthermore, the notes are presented using a "Post-it" template, defined as:
<prism:VisualStateAwarePage.Resources>
    <DataTemplate x:Name="PostIt">
        <Grid Width="250" Height="250" >
            <TextBlock 
                    FontFamily="Buxton Sketch" 
                    Text="{Binding Text}" 
                    FontSize="20" 
                    HorizontalAlignment="Left" 
                    VerticalAlignment="Top" 
                    Margin="20,40,50,0" 
                    TextWrapping="WrapWholeWords">
            </TextBlock>
        </Grid>
   </DataTemplate>
</prism:VisualStateAwarePage.Resources>
And finally the GridView with the properties for the ItemsSource and the Template:
<GridView x:Name="grdMessages"
       ItemsSource="{Binding Messages}"
       ItemTemplate="{StaticResource PostIt}">
</GridView>
So far for the XAML for the presentation in the View. The codebehind is nothing but a simple constructor with Initialization, it looks like:
Namespace Views
    Public NotInheritable Class BoardPage
        Inherits VisualStateAwarePage
        Public Sub New()
            InitializeComponent()
        End Sub
    End Class
End Namespace
This is all pretty straightforward, thankfully using the Prism library to get richer functionality. The next part is the ViewModel, and also the Viewmodel is straightforward, out of the book.
It looks like:
Namespace ViewModels

    Public Class BoardPageViewModel
        Inherits ViewModel

        Private _MessageRepository As IMessageRepository
        Private Property _Messages As ObservableCollection(Of Message)
        <RestorableState>
        Public Property Messages As ObservableCollection(Of Message)
            Get
                Return _Messages
            End Get
            Set(value As ObservableCollection(Of Message))
                SetProperty(_Messages, value)
            End Set
        End Property

        Public Overrides Async Sub OnNavigatedTo(navigationParameter As Object, navigationMode As NavigationMode, viewModelState As Dictionary(Of String, Object))
            MyBase.OnNavigatedTo(navigationParameter, navigationMode, viewModelState)
            _MessageRepository = New MessageRepository
            Messages = Await _MessageRepository.GetMessagesAsync()
        End Sub

    End Class
End Namespace
The messages are retrieved via a WebAPI that is working fine. When I step through the code, I see the Messages arriving and the Set, and thus the SetProperty are called.

But the Messages do not show up on the View. It seems to me that the View is not notified of the change, when the Messages have been retrieved.

What is wrong with this approach?
Sep 2, 2014 at 9:41 AM
Edited Sep 2, 2014 at 9:43 AM
Maybe your compiler does not support [CallerMemberName] hence you need to call
SetProperty(_Messages, value, "Messages")
Or maybe the DataContext isn't set properly. Check your ouput window for binding errors
Sep 2, 2014 at 10:20 AM
Edited Sep 2, 2014 at 10:25 AM
You are missing the ref on your SetProperty of messages.
You may also be missing the Mode part, Text="{Binding Text, Mode=TwoWay}", by default I think the Mode is OneWay.
Sep 2, 2014 at 12:08 PM
I wonder how it does compile without the ref keyword?! BindableBase.SetProperty<T> requires it?!
Sep 2, 2014 at 1:31 PM
Must be a VB thing. Not sure myself.
Sep 2, 2014 at 11:16 PM
@ViktorHofer, @Allann, Thanks for your replies. I will comment on your suggestions in one combined reaction.

I have no influence on what the compiler does or can't do. My development environment is VS2013.3 Pro edition.

Based on your suggestion I have added the extra name "Messages" and I have also tried "_Messages" in the SetProperty command. That does not influence the behaviour of the process in BindableBase. So when I step through teh code, I see exacty the same sequence, namely that the Get _Messages is called BEFORE the _Messages have the new value.

By the way: I think the extra third parameter is optional with a default of null. I see no use of that third parameter in the AdventureWorks reference application.

The BindableBase sees that the storage differs from the value, and so it calls OnPropertyChanged("Messages"). Next the event is handled, but that is too late in my impression. The call to the property-Get has been done by then.

I see no binding errors in the output window.

I have tried {Binding Text, Mode=TwoWay} -> no different behaviour.
Sep 3, 2014 at 12:05 AM
Edited Sep 3, 2014 at 12:13 AM
As Allann already suggested, you need to pass the _Messages object as reference (ByRef keyword in visual basic).

I will explain the situation to you in more detail. BindableBase has the following algorithm executed in the SetProperty function:
protected virtual bool SetProperty<T>(ref T storage, T value, [CallerMemberName] string propertyName = null)
        {
            if (object.Equals(storage, value)) return false;

            storage = value;
            this.OnPropertyChanged(propertyName);

            return true;
        }
So it first checks if the old and new value are equal and if yes it stops executing and returns. If those two objects are not equal, then on the memory address of the old value (storage) the address to the new value will be written --> pointer.
Your problem could be the following now: You do not pass the old value (storage) as a reference but you are accidentally passing it as a copy. So the copy of the old value will be overwritten with the new value and then the PropertyChanged Event is triggered. Then the GUI is calling the getter of your old property and will get the old value.

So my advice ---> Try passing the existing, old value as reference to the SetProperty function with the ByRef keyword.

Additional guidance: http://msdn.microsoft.com/en-us/library/ddck1z30.aspx
Sep 3, 2014 at 7:57 AM
Hi all readers!

In an attempt to make it more attractive and more clear, I have created a small video capture of what is happening, how it is implemented and what the reaction in the view looks like. Please take a look at: this recording
Perhaps you will have to turn up the sound a bit extra, because the recording was not made very loud.

I hope to get back to receive back some valuable tips and suggestions!

Regards!
Sep 3, 2014 at 10:02 AM
Can you please confirm what is coming from your Repository? I'm assuming it isn't an observable collection. At least it shouldn't be, I'd think it would be a serialisable POCO object. This means that you will need to add the items to the observable collection. The way I do this if not using a CollectionView is to have a readonly variable of type ObservableCollection<MessageViewModel>, which is never changed. Then when you make the request to messages, I would ensure the collection is cleared, ready for new the new items. Once the messages are returned, loop through each model item (message) within the response and convert them to MessageViewModels (a new class that contains bindable properties and validation (data annotations) as required. As each ViewModel item is created it is added to the observablecollection. The act of adding items to the collection will raise an event the listview is listening for and therefore will display the item (as long as the MessageViewModel has an associated data template.

private readonly _messages = new ObservableCollection<MessageViewModel>();

Public ObservableCollection<MessageViewModel> Messages {get { return _messages;}}

OnNavigateTo

Messages.Clear
foreach(var message in await _messageRepository,GetMessagesAsync())
{
Messages.Add(new MessageViewModel(){Name = message.Name, Text = message.Text});
}

Does that make sense?
Sep 3, 2014 at 10:20 AM
Have you already tried what I suggested in my last post?

And one obvious mistake in your video, if you call SetProperty(...,...,"_Messages") is wrong. You have to enter the name of the PUBLIC property so SetProperty(...,...,"Messages"). But I'm 100% sure that you can leave it out, because you are using visual studio and it detects the name of the calling function automatically with CallMemberName.
Sep 3, 2014 at 11:32 AM
Hi ViktorHofer and Allann,

Again, I will answer both of your suggestions in one reply.
First of all, the Object coming from the repository is an ObservableCollection (of Message). I will try working with the rest of your suggestion to add a new local object as ObservableCollection and add the items from the received messagescollection in there. It looks completely redundant to me, but then again: if that would make the boat to go afloat...

I will let you know the results afterwards.

As for the name of the extra optional parameter: in the video you see the results of the experiment with the '_Messages" name, but trust me, I have been trying without the parameter, with the "Messsages" and with the "_Messages" all with the same result. Because the last variant was with the underscore, that is the one that ended up in the recording. But really be assusred that there's no difference in behaviour!

Thanks for your patience.
Sep 3, 2014 at 12:16 PM
Hi Allann and ViktorHofer,

Finally it works! To show and prove it, again I have recorded a small video to show the implementation.

And to say out loud: THANK YOU!

There is a 50 point bounty for this solution on Stackoverflow, I think Allann deserves it. So if you're interested it's here.

Regards
Sep 3, 2014 at 12:32 PM
Hi,

I could not stand the fact that when the repository already returns an observablecollection, why it would be necessary to have another object and then passing the elements one by one.
Stepping through the code of the last video, I noticed that SetProperties was NEVER called during the For Each loop. If that is the case, then we can leave it out just as well. So that is what I tried. Then the Public Property need to be instantiated "by itself" at construction time of the ViewModel. So that is what I did.

Finally I discovered that "injecting" that newly created Messages object into the repository, it would get loaded with data immediately from there (it being an observablecollection anyhow). And guess what: it keeps working stable. And with a lot less code.

Here 's what is left over from all our experimentations:
    Public Class MessagesPageViewModel
        Inherits ViewModel

       Private _MessageRepository As IMessageRepository
       Public Property Messages As new ObservableCollection(Of Message)

        Public Overrides Async Sub OnNavigatedTo(navigationParameter As Object, navigationMode As NavigationMode, viewModelState As Dictionary(Of String, Object))

            MyBase.OnNavigatedTo(navigationParameter, navigationMode, viewModelState)

            _MessageRepository = New MessageRepository
            Await _MessageRepository.GetMessagesAsync(Messages)

        End Sub
    End Class
BUt still... I could only come to this solution via the roundtrip with Allann's suggestion. So you get the credits!
Sep 3, 2014 at 1:11 PM
Glad you found a result. I'll explain my answer a little more regarding the layers.

Server side has the data stored somewhere, file, db, etc. This data is retrieved and passed the the client via a serialisable POCO class via some type of service (WebApi, OData, WCF, etc). The client will normally receive this as an IEnumerable<T> and therefore needs to be made into a INPC class that can can be bound too, Hence my suggested solution.

Regardless, happy you found a solution for your problem.
Sep 3, 2014 at 1:39 PM
Edited Sep 3, 2014 at 1:40 PM
I still think that the core problem is that you are not passing the object as a reference to the SetProperty function (ByRef)! And as Allann already explained, i would not create an ObservableCollection in the DataAccessLayer, normally I return an IReadOnlyCollection<T> or simply an IEnumerable<T> to the upper layer.
Sep 3, 2014 at 1:48 PM
Thanks Allann, thant makes sense to me. The conversion from the straightforward List into an ObservableCollection has been done in my Repository already. I implemented the repository like so:
       Public Async Function GetMessagesAsync(Messages As ObservableCollection(Of Message)) As Task(Of ObservableCollection(Of Message)) Implements IMessageRepository.GetMessagesAsync

            Dim MyMessage As New Message  'there are some API helper settings in this Messsage
            Dim DatabaseObjects As New List(Of Message)

            Dim MyAPIResponse As HttpResponseMessage = Await MyMessage.APIGet_Response()

            If MyAPIResponse.IsSuccessStatusCode Then
                Try
                    Dim AnswerFromAPI = Await MyAPIResponse.Content.ReadAsStringAsync
                    DatabaseObjects = JsonConvert.DeserializeObject(Of ObservableCollection(Of Message))(AnswerFromAPI)

                    For Each FoundMessage In DatabaseObjects
                        Messages.Add(FoundMessage)
                    Next

                Catch ex As Exception
                    'nothing yet,, errormessages to be implemented later
                End Try

            End If
            Return Messages
        End Function
So thanks for this explanation of layers, I see why it is possible that my final solution keeps working. I had transferred the elements of the DatabaseObjects into the Messages already in the repository.