CODEWITHSUNDEEP
c#wpfmvvm
Steve
Steve

Reputation: 1694

Where do I call the model in the ViewModel?

I'm learning MVVM.

I have my View filling two comboboxes from ObservableCollection properties in my ViewModel (eg. properties "Oc1" & "Oc2"). I also have a property bound to the selected item of Oc1 (eg. property "SelVal") that Oc2 depends on, so when property SelVal is changed, Oc2 needs to re-get it's data from the database.

Now, I've come up with a solution and it works for my situation but doesn't seem to adhere to the principle of a get accessor so I'd like to know what problem might I face down the track and what is a better solution?

My current solution is:

The get accessor of Oc2 queries the database and sets it's private field to the value returned from the database (which the View uses). So when SetVal is changed, I simply call this.RaisePropertyChanged("Oc2") in the SetVal set accessor and the View asks for Oc2, which in turn queries the database and returns the updated list. The problem is that I'm not using the get accessor for what it is meant for, as I'm assigning it's value in it. But what I like about it is it's self-contained (ie. I don't need a "BindOc2" method which I'd have to call in the constructor and then again in the SelVal set accessor). Please advise. And what's a better way?

Upvotes: 3

Views: 209

Answers (5)

GazTheDestroyer
GazTheDestroyer

Reputation: 21261

I understand your reticence since yes, raising PropertyChanged for a different property feels a bit hacky, but it's not too bad IMHO.

A more natural way would be to do your database pull in the SelVal setter, since this is what is triggering the change of data. You then set Oc2 to the results which will automatically raise a PropertyChanged as it should.

The only problem with this is that you might be pulling db results unnecessarily if your Oc2 getter is never accessed, but given that you know your view will always want them I'd be tempted to change to this solution.

Upvotes: 2

Panagiotis Kanavos
Panagiotis Kanavos

Reputation: 131712

Your suspicion is correct, this way breaks the MVVM model and fails to use mechanisms that can simplify your work, like the triggers in System.Windows.Interactivity which is available in the Expression Blend SDK.

Loading the data in the getter is begging for trouble. It binds your model to the data access code, makes testing harder and the code more complex by mixing different concerns in the same property. Besides, Murphy's law dictates that at some point you will just want to set the property without reloading from the database.

A better solution is to extract the data loading code in a separate method that can be called by a command or a trigger. This method will either modify the Oc2 collection's contents or simply replace it with a new collection. In any case, the property setter will raise the proper notification and the second combo will be updated.

The following example comes from fabien's answer in a similar SO question:

<ComboBox x:Name="fileComboBox" ItemsSource="{Binding FileList, Mode=TwoWay}">                  
        <i:Interaction.Triggers>
            <i:EventTrigger EventName="SelectionChanged">
                <i:InvokeCommandAction 
                        Command="{Binding SelectionChangedCommand}"
                        CommandParameter="{Binding SelectedItems,           
                        ElementName=fileComboBox}"/>
            </i:EventTrigger>
        </i:Interaction.Triggers>

    </ComboBox>

Most frameworks provide support for this design. Caliburn.Micro provides actions that will call a ViewModel method when a UI event occurs, and a shortcut syntax to avoid writing the trigger XAML and the corresponding command. It simply connects the event to a ViewModel method. The XAML can be as simple as this:

<ComboBox x:Name="Oc1" 
    cal:Message.Attach="[Event SelectionChanged] = 
                        [Action ReloadFor($this.SelectedItem)]" />

The $this value is another shorthand that references the ComboBox itself.

If you don't want to use triggers, you can bind the SelectedItem property of your combo to a ViewModel property and execute the Reload method whenever this property changes, eg.:

<ComboBox ... SelectedItem={Binding CurrentOC2,Mode=TwoWay} />

Upvotes: 2

Mare Infinitus
Mare Infinitus

Reputation: 8192

If those data are not changing, you could have that in your viewmodel.

Always querying the database should not be the first option.

you could do this in a quite simple way:

Dictionary<string, List<string>> cache;
...
List<string> subCat;
if cache.TryGetValue (selVal, out subCat) 
{
    // no need to call database
}
else
{
   // else call database
   // insert result in cache
}

Upvotes: 1

Kenneth Ito
Kenneth Ito

Reputation: 5261

So to rephrase you have something similar to Category, CurrentCategory, and SubCategory. When CurrentCategory changes, SubCategory needs to refresh.

I think your way is fine. Especially in MVVM I see this type of thing all over the place. Outside of MVVM it's fairly common to have property getters hitting the database for lazy load scenarios (usually invoking some sort of a service method, not inlining everything).

Upvotes: 1

fenix2222
fenix2222

Reputation: 4730

The way you do it is fine. Properties set and get methods are usualy good for this kind of Binding

Upvotes: 1

Related Questions