CODEWITHSUNDEEP
c#wpfmvvmmvvm-light
Ivan Zaporozhchenko
Ivan Zaporozhchenko

Reputation: 333

MVVM is command in model a bad practice?

I'm wondering, is putting command into model is bad practice in MVVM. For example, I have ListBox with Image and Button. When I click on Button I need to open url in browser. So, my code will look like this:

<ListBox ItemSource="{Binding Items">
    <ListBox.ItemTemplate>
        <DataTemplate>
            <StackPanel>
                <Image Source={Binding ImageSource} />
                <Button Content="Open url" Command={Binding OpenUrlCommand}"/>
            </StackPanel>
        </DataTemplate>
    </Listbox.ItemTemplate>
</ListBox>

ViewModel:

class MainViewModel:BaseViewModel
{
    public ObservableCollection<Model> Items {get;set;}
}

Model:

class Model
{
    public string ImageSource {get;set;}

    public string Url {get;set;}

    public ICommand OpenUrlCommand {get;set;}

    public Model()
    {
       OpenUrlCommand = new RelayCommand(openUrl);
    }

    public void openUrl()
    {
        Process.Start(Url); //Open url in browser
    }
}

Is it ok, or I should move OpenUrlCommand to MainViewModel ?

Upvotes: 1

Views: 1929

Answers (1)

heltonbiker
heltonbiker

Reputation: 27575

You actually should implement the method in the Model, but the command in the ViewModel. This would be much more aligned to the MVVM architecture, and would be no additional work at all for you in this case.

In your ViewModel:

class MainViewModel : BaseViewModel
{
    private Model _model;    

    ICommand OpenUrlCommand { get { return new RelayCommand(_model.openUrl); } }

    // ...
}

If you want to "send" some URL from the View, you can use CommandParameter for that, and have a typed RelayCommand.

UPDATE: Note that, since the DataTemplate where the Command binding resides is the ItemTemplate, you should implement the Command in some ItemViewModel, not in the MainViewModel. Then, make Items an ObservableCollection<ItemViewModel> instead of ObservableCollection<Model>, and initialize the collection like this:

IEnumerable<Model> models= getSomeModelsToStartWith();
var Items = new ObservableCollection<ItemViewModel>(models.Select(m => new ItemViewModel(m));

Upvotes: 6

Related Questions