MVVM RaisePropertyChanged in ViewModel or Model?

Question

let's say I have a Server class with several properties (such as Ping) which are updated in the background. I have a ServerViewModel which is holding the Server instance and a ServersViewModel which is holding the different instances of ServerViewModel in a ObservableCollection to display the servers in a List in the View.

The (shortened) Server Class:

public class Server: ObservableObject, IServer 
{

    /// <summary>
    /// Gets the IP adress of the server
    /// </summary>
    [JsonIgnore]
    public IPAddress IPAdress { get; private set; }

    /// <summary>
    /// Gets the name of the server
    /// </summary>
    public string Name
    {
        get
        {
            return name;
        }

        private set
        {
            if(name != value)
            {
                name = value;
                RaisePropertyChanged("Name");
            }

        }
    }

    /// <summary>
    /// Gives the string of the ip
    /// </summary>
    public string IP
    {
        get
        {
            return ip;
        }

        private set
        {
            if(ip != value)
            {
                ip = value;
                RaisePropertyChanged("IP");
            }                
        }
    }


    /// <summary>
    /// Gets the ping of the server
    /// </summary>
    [JsonIgnore]
    public int Ping
    {
        get
        {
            return ping;
        }

        private set
        {
            if(ping != value)
            {
                ping = value;
                RaisePropertyChanged("Ping");
            }
        }
    }

    private Thread queryThread;

    private string name, ip;

    /// <summary>
    /// Initializes the server instance
    /// </summary>
    /// <param name="ip">The ip adress or dns of the server</param>
    /// <param name="port">The common port to connect to</param>
    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2214:DoNotCallOverridableMethodsInConstructors")]
    public Server(string ip, int port)
    {
        IP = ip;

        IPAddress tmpIP;

        // Check if the given IP matches a DNS
        if (!IPAddress.TryParse(ip, out tmpIP))
        {
            try
            {
                this.IPAdress = Dns.GetHostEntry(ip).AddressList[0];
            }
            catch (Exception ex)
            {
                System.Diagnostics.Debug.WriteLine(ex);
                return;
            }
        }
        else
        {
            this.IPAdress = IPAddress.Parse(ip);
        }

        Port = port;

        queryThread = new Thread(QueryServer);
        queryThread.Start();
    }

    /// <summary>
    /// Destructor for Server class. Stops the server thread running in background and cleans up.
    /// </summary>
    ~Server()
    {
        try
       {
            queryThread.Abort();
            queryThread = null;
        }
        catch
        {
            // Ignored
        }
    }

    /// <summary>
    /// Starts the loop that is querying the server
    /// </summary>
    private void QueryServer()
    {
    }
}

As I'm updating the data in the background I added RaisePropertyChanged in the Proerties where it is needed.

The ServerViewModel:

public class ServerViewModel: ViewModelBase
{
    private Server server;

    public Server Server
    {
        get
        {
            return server;
        }
    }

    public ServerViewModel(Server server)
    {
        this.server = server;
    }
}

The ServersViewModel:

public class ServersViewModel: ObservableObject
{
    #region Members

    ObservableCollection<ServerViewModel> servers = new ObservableCollection<ServerViewModel>();

    #endregion

    #region Properties
    public ObservableCollection<ServerViewModel> Servers
    {
        get
        {
            return servers;
        }

        set
        {
            servers = value;
        }
    }
    #endregion

    #region Construction
    public ServersViewModel()
    {
        // Add servers to the collection
    }
    #endregion
}

I'm binding the ServersViewModel to a list in the view. My DataBinding for each item in the list looks like this:

<Label  Name="lblServerPing" Content="{Binding Server.Ping}" />

For me this looks wrong. Especially because I'm accessing the Server instance of the view and not a property of the view. I still don't know where to put the RaisePropertyChanged and how to fix this behaviour. I have to admit that it is working this way but I think it is not how it should look like in MVVM.

ObservableObject and ViewModelBase are from MVVM light and are implementing the needed interfaces.

Thanks for your help!

Answer 1

Typically the PropertyChanged event should be raised in the ViewModel, and the model should just be a collection of properties/objects. The ViewModel would then expose those properties and notify the view when they change through the PropertyChanged event.

Here's a simplified version of what you're doing, which should hopefully paint a picture of where to go, IF you want to follow MVVM strictly, that is...

public class ServerModel
{
    public string Name { get; set; }
    public string IP { get; set; }
}

public class ServerViewModel : ViewModelBase
{
    private ServerModel model;

    public ServerViewModel(ServerModel model)
    {
        this.model = model;
    }

    public string Name
    {
         get { return model.Name; }
         private set
         {
              model.Name = value;
              OnPropertyChanged("Name");
         }
    }

    public string IP
    {
         get { return model.IP; }
         private set
         {
              model.IP= value;
              OnPropertyChanged("IP");
         }
    }
}

The validation logic that you have inside the setters, would go in the ViewModel. If logic inside the ViewModel becomes complex, it should ideally be outsourced to a service class - the ViewModels shouldn't contain a lot of complex logic, they simply tell the Views how to display the Model data.

Hope this helps.