Hierarchical data problems in MVVM.

Question

Do you think this Model-ViewModel implementation is correct? Note that Stars (in the VM) is an ObservableCollection of VMs inside another one, and i feel like it decouples the VM from the M because when removing an element from Stars, i still have to delete it's model manually.

Any ideas on how to improve this without using OnCollectionChanged? Thanks in advance.

Models:

public class Galaxy
{
    public Galaxy(string name, IList<Star> stars)
    {
        Name = name;
        Stars = stars;
    }
    public string Name { get; set; }
    public IList<Star> Stars { get; set; }
}

public class Star
{
    public Star(string name)
    {
        Name = name;
    }
    public string Name { get; set; }
}

ViewModels :

public class GalaxyVM : ViewModelBase
{
    private Galaxy _galaxy;
    private ObservableCollection<StarVM> _stars;

    public GalaxyVM(Galaxy galaxy)
    {
        _galaxy = galaxy;
        _stars = new ObservableCollection<StarVM>(from sys in _galaxy.Stars
                                                  select new StarVM(sys));
    }

    public string Name
    {
        get { return _galaxy.Name; }
    }
    public ObservableCollection<StarVM> Stars
    {
        get { return _stars; }
    }
}

public class StarVM : ViewModelBase
{
    private Star _star;
    public StarVM(Star star)
    {
        _star = star;
    }
    public string Name
    {
        get { return _star.Name; }
    }
}

Answer 1

Well, there's decoupling and there's decoupling.

It's one thing to have a model that doesn't know anything about the implementation details of its views. That's good.

It's another thing to have a model that can't be used in dynamic views. If a model class doesn't implement change notification, it's going to be difficult to reliably keep the UI in sync with the state of the model. You can try and phony up change notification by putting it all into your view classes, but at the end of the day there's going to be code somewhere that changes the state of the model directly, and if it doesn't implement change notification the views aren't going to know about it and the UI will get out of sync.

I probably wouldn't implement an IList<Star> property with a public setter in Galaxy. I'd make the Stars property of type StarList, and create a StarList class that implemented IList<Star>, unless I had a very good reason not to.