CODEWITHSUNDEEP
c#.netwpfmvvmviewmodel
LuckyStrike
LuckyStrike

Reputation: 1493

WPF MVVM design ViewModel

I have the following model:

public class Person
{
    public string LastName{get;set;}
    public City City {get;set;}
}

public class City
{
    public string Name {get;set;}
}

I have two Views:

My AllPersonsViewModel:

public class AllPersonViewModel : ViewModel
{
    public ObservableCollection<PersonViewModel> PersonViewModels {get;set;}
}

I started with the following PersonViewModel:

public class PersonViewModel : ViewModel
{
    private Person _person;

    public string Name
    {
        get { return _person.Name;}
        set { _person.Name = value; RaisePropertyChange("Name");}
    }

    public string CityName
    {
        get { return _person.City.Name;}
    }
}

Then I added the properties for adding a new Person. In the View I need a Textbox for the PersonName and a Combobox for selection of a City:

public class PersonViewModel : ViewModel
{
    private Person _person;

    public string Name
    {
        get { return _person.Name;}
        set { _person.Name = value; RaisePropertyChange("Name");}
    }

    public string CityName
    {
        get { return _person.City.Name;}        
    }

    public City SelectedCity 
    { 
        get { return _person.City;}
        set { _person.City = value; RaisePropertyChange("SelectedCity");}
    }

    public ObservableCollection<City> Cities {get;set;} 
}

Is this the right approach? It seems a little bit redundant to me. In the Grid of AllPersonsView I could also bind directly to the "SelectedCity.Name" instead of the extra property CityName. The grid is also readonly.

Upvotes: 1

Views: 466

Answers (2)

Martin Liversage
Martin Liversage

Reputation: 106926

Your question really boils down "is it OK to expose my model directly to the view?" Some purist will say no while other will say that having a view model that wraps a model without adding any new functionality is redundant.

In my opinion it depends on the task at hand but "skiping" a view model may come back and bite you later when you need to add additional state that doesn't belong in the model. If in doubt use a view model but for instance when exposing simple model objects in a list you often don't need the extra layer the view model provides.

In your case you have opted for the "purist" solution and because your model object doesn't support INotifyPropertyChanged you can't get rid of the view model if a model property is changed by multiple sources. But instead of providing a CityName property you could bind to SelectedCity.Name. WPF supports property navigation in data binding expressions.

For more insight into this topic you can google mvvm expose model.

Upvotes: 1

daryal
daryal

Reputation: 14929

you have multiple problems;

1 - you do not need to declare an observable collection of viewmodels in AllPersonViewModel. Just declare an ObservableCollection of Person.

2 - do not add the CityName property; not needed as you have stated.

3- do not add the Name property. Bind the textbox to Name property of the Person.

Upvotes: 2

Related Questions