1

I have a couple of questions regarding MVVM view models. I have 3 view models in my scenario, which I have put together a shorter sample below. I was looking for a nice solution for 3 views working in conjunction with one another in a small part of a large application. There are 3 classes in play here:

ParentViewModel

Represents a view that presents a combo box with source of MyLookups to the user to select a value and store in SelectedLookup.

There is a button which will invoke the ShowDialogCommand which will show the DialogVM1 via the IDialogService.

DialogViewModel

Represents a view where the user will enter some field information into NewItemTemplate. When they click OK the dialog will close and the NewItemTemplate is passed to ChildVM1 by calling SetSomeInformation.

ChildViewModel

Within ChildViewModel the template data is then transformed into a list of data but this is irrelevant. The ChildViewModel provides a grid where the user can modify a list of SomeData. The DefaultItem Lookup is populated from a combo in the grid who's data source is MyLookups.

IViewModelLocatorService

This uses my IoC container to get the view instance. The views themselves have a variable number of service dependencies. The purpose of the IViewModelLocatorService was to create my view instance when I needed it. However, whilst writing this post, I am thinking I could just inject the view instance into ParentViewModel constructor. All of my view models are registered in the IoC container.

ILookupService

Its purpose is to populate a list of items which will be displayed as a combo box on all 3 views. The FindAll will basically hit the database (calling Entity Framework) and populate a new list. This creates a new DbContext each time. At the moment Lookup does not override Equals so although the data will be the same based on Id, but the x == y would be false - so where the data is being passed around, I am working with 3 different lists.

Currently all services are registered as a singleton.

public class ParentViewModel
{
    private readonly ILookupService _lookupService;
    private readonly ISomeService _someService;
    private readonly IViewModelLocatorService _vmLocatorService;
    private readonly IDialogService _dialogService;

    public ParentViewModel(
        ILookupService lookupService,
        ISomeService someService, 
        IViewModelLocatorService vmLocatorService,
        IDialogService dialogService,
        ChildViewModel1 ChildViewModel              // should we inject this?
        )
    {
        ...
        MyLookups = _lookupService.FindAll(); // actually done from a load command but added here for simplicity sake
    }

    public ChildViewModel1 { get; private set; }

    public ObservableCollection<Lookup> MyLookups { get; set; }

    public Lookup SelectedLookup { get; private set; }

    public ICommand ShowDialogCommand => new RelayCommand(
                    execute: () => 
                    {
                        // I want to show DialogViewModel here
                        var dialogVM = new DialogViewModel(...);
                        var dialogVM = _vmLocatorService.NewInstance<DialogViewModel>();
                        dialogVM.DefaultLookup = this.SelectedLookup;

                        _dialogService.Show(dialogVM, this);

                        if (dialogVM.DialogResult == true)
                        {
                            // pass some information to the child VM
                            ChildViewModel1.SetSomeInformation(dialogVM.NewItemTemplate);
                        }
                    },
                    canExecute: () => true);

}

public class ChildViewModel1
{
    private readonly ILookupService _lookupService;

    public ChildViewModel1(ILookupService lookupService)
    {
        ...
        MyLookups = _lookupService.FindAll();
    }

    public void SetSomeInformation(Lookup newLookupSelection)
    {
        ...
    }

    public ObservableCollection<Lookup> MyLookups { get; set; }
}

public class DialogViewModel
{
    private readonly ILookupService _lookupService;
    private readonly ISomeOtherService1 _someOtherService1;
    private readonly ISomeOtherService2 _someOtherService2;

    public DialogViewModel(ILookupService lookupService, ISomeOtherService1 someOtherService1, ISomeOtherService2 someOtherService2)
    {
        ...
        MyLookups = _lookupService.FindAll(); // actually done from a load command but added here for simplicity sake
    }

    public SomeData NewItemTemplate { get; private set; }

    public ObservableCollection<Lookup> MyLookups { get; set; }
}

public class SomeData
{
    public long Id { get; set; }
    public string Name { get; set; }
    public Lookup DefaultItem { get; set; }
    ...
}

My questions are:

  1. Is IViewModelLocatorService a reasonably valid way to get a view model instance in a view model or is it best to keep the dependency explicit by injecting the child view model as a constructor dependency? I've read things about this being a bad practice in the past although I can't find anything after a quick look tonight.

  2. With the ILookupService, there is currently 3 different lists - each with pretty much the same data. Is there a nice way to make the 3 views use the same list of data? The only idea I have is to forcefully set the list on the DialogVM and ChildViewModel via a property.

  • Questions asking for assistance in writing or debugging existing code are off-topic on Programmers. In addition, I read ParentVM as "parent virtual machine". Stick to the well-established convention of ParentViewModel and your code will be much easier for others to read. – David Arno Jul 28 '16 at 20:20
  • I did originally post on SO but deleted that after been recommended to post on here. ParentVM was the name of the class in the sample – Andez Jul 28 '16 at 20:29
  • 2
    OK, I can see that must be frustrating to be passed from one "not here" site to another. Service locators are anti-patterns. Injecting a service locator smacks of a code smell to me as it's not injecting what the class needs, rather injecting the world into it and leaving it to figure out what it might need. So in answer to (1), inject the minimum required. For example of why, see Mike Hadlow's post on why not to inject containers – David Arno Jul 28 '16 at 20:38
  • With regard to (2), without more code (which would make your question more of a violation of the site! (you can't win)) it's difficult to say. Gut feeling is that again only injecting what's needed should solve this, but I can't be sure. – David Arno Jul 28 '16 at 20:41
  • 2
    @DavidArno: The presence of code, in and of itself, doesn't make a question off-topic here. Mostly, what we're trying to avoid is "fix my broken code" and "write my code for me" questions. That doesn't mean that posters can't include code for illustrative purposes. – Robert Harvey Jul 28 '16 at 20:59
-1

Always aim for reducing cognitive load and handing over verification to compiler.

Specifically:

  • Avoid IoC containers and service locators because they shift errors to runtime.
  • Keep everything immutable because you will always know that sharing references will not do any harm.

In practice:

  1. Inject all dependencies using the pure, static-analysis-friendly, language-integrated DI aka the new operator. Unless of course you are dealing with a highly dynamic application (e.g. plug-in support, run time reconfiguration, hot loading etc.) or there are other build configuration or dependency management considerations.

  2. If the data is static then create an IProvider with a lazy All read-only collection property populated once from injected ILookupService. Then inject the same instance of IProvider in three places. If the data is dynamic then apply http://dynamic-data.org as well to make the property reactive.

  • 1
    A downvote from a container cargo cult fanboy I guess :). – Den Aug 1 '16 at 8:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.