How to observe nested objects

Question

By having two model classes Conversation and Message, what are the best practices to handle the next situation: A conversation listening for its messages PropertyChanged events and so being able to update itself.

1. What are the best practices?
2. How can I improve this model design?
3. Is it going to generate memory leaks?

code

using SoftConsept.Collections;

public class Conversation 
{
    readonly SortedObservableCollection<Message> messages;

    public Conversation ()
    {
        messages = new SortedObservableCollection<Message> ();
    }

    public void Add (Message message)
    {
        messages.Add (message);
        message.PropertyChanged += HandleMessagePropertyChanged;
    }

    public void Remove (Message message)
    {
        message.PropertyChanged -= HandleMessagePropertyChanged;
        messages.Remove (message);
    }

    public IList<Message> Messages ()
    {
        return messages.ToList ();
    }

    void HandleMessagePropertyChanged (object sender, PropertyChangedEventArgs e)
    {
        // Uptade omitted conversations properties using data from the updated message.
    }
}

public class Message : INotifyPropertyChanged 
{
}

Answer 1

First of all, the Conversation class does not provide any events for chances, so nobody has a chance to get to know if something changes. This is already be implemented within an (Sorted)Observable collection, but unfortunately you publish this private field as IList<Message> by the Message method. So the notification abilities of this collection can not be used. Furthermore methods should always be connected with a action, so GetMessages() would be a more suitable name. But the best way is to make it a property.

using SoftConsept.Collections;

public class Conversation : INotifyPropertyChanged
{
    readonly SortedObservableCollection<Message> messages;

    public event PropertyChangedEventHandler PropertyChanged;

    public Conversation ()
    {
        messages = new SortedObservableCollection<Message> ();
    }

    private DateTime _updateTime = DateTime.Now;
    public DateTime UpdateTime{
       get{ return _updateTime;}
       private set{
           UpdateTime = value;
           OnPropertyChanged("UpdateTime");
       }
    }

    private void OnPropertyChanged(string propertyName){
       var handler = PropertyChanged;
       if(handler==null) return;
       handler(this, new PropertyChangedEventArgs(propertyName));
    }

    public void Add (Message message)
    {
        messages.Add (message);
        UpdateTime = DateTime.Now;
        message.PropertyChanged += OnMessagePropertyChanged;
    }

    private void OnMessagePropertyChanged(object sender, PropertyChangedEventArgs args)
    {
        UpdateTime = DateTime.Now;
    }

    public void Remove (Message message)
    {
        messages.Remove (message);
        UpdateTime = DateTime.Now;
        message.PropertyChanged -= OnMessagePropertyChanged;
    }

    public ObservableCollection<Message> Messages 
    {
        get{ 
            //I assume that SortedObservableCollection is subtype of ObservableCollection
            return messages;
        }
    }
}

public class Message : INotifyPropertyChanged {}

So now some observer might register for changes in Messages (add, delete) and also for changes in the single Message instances.

For more info, look at Microsoft documentation:

Answer 2

There are two possible memory leaks likely to occur.

• You should implement the IDisposable interface or provide an alternative way of deattaching an object from events subscribed to referenced dependencies and/or event listeners.
• You should let the container objects dealing with Converstation and Message instances handle the lifetime of these objects correctly.

Conversation could keep Message instances alive and vice versa.

public void Clear() 
{
    foreach (var message in messages) 
    {
        message.PropertyChanged -= OnMessagePropertyChanged;
    }
    messages.Clear();
}

public void Dispose()
{
    // .. dispose pattern impl
    Clear();
}

Message could keep any listener alive and vice versa.

public void Reset() 
{
    PropertyChanged = null;
}

public void Dispose()
{
    // .. dispose pattern impl
    Reset();
}

Consider using Weak Event Pattern if you don't want a strong reference between an object and its event listeners.