-1

In my WPF MVVM project I have a few forms and a database that holds employee information. There is an employee view model, this has an employeeId passed to it, using the employee model, the employee's details are read from the database. This also reads if the employee is a 'User' or 'Admin'.

The view model will then decide which view to the load. The user view or the admin view. There are lots of similarities between the 2 views which is why I'm using the same view model for both.

This is why for my project I decided to be consistent and have my view models open their views. However, researching online I can only see advice saying to instantiate the view, which will then create the view model in the view constructor.

Is my method the wrong way for MVVM - and how would I do this scenario the right way?

MainWindowViewModel:

    public void OpenSelectedEmployee(object employee)
    {
        if (employee == null)
            return;

        StaffListModel model = (StaffListModel)employee;

        EmployeeViewModel vm = new EmployeeViewModel(model.EmployeeId);
    }

EmployeeViewModel:

    private EmployeeModel employee;
    private Window employeeWindow;
    
    // Employee View Model Constructor
    public EmployeeViewModel(int employeeId)
    {
        // Connect to DB and read data for employee class
        if (LoadEmployee(employeeId))
        {
            if (employee.UserType == "User")
                employeeWindow = new EmployeeUserDetailsView();
            else // if Admin
                employeeWindow = new EmployeeAdminDetailsView();
            
            employeeWindow.DataContext = this;
            employeeWindow.ShowDialog();
        }
        else
        {
            // Open error message
            MessageBox.Show("Error loading employee occurred");
        }
    }

I have considered storing the user type in the main window view model, so that decides which view to open. But if for any reason the database fails to read the employee (LoadEmployee(employeeId)), there's no point creating the view and I would like the error message box to show.

Edit: Here is the LoadEmployee method currently in EmployeeViewModel. It is used in the constructor and by a binding button to refresh the form.

    public bool LoadEmployee(int employeeId)
    {
        if (!DbConnector.OpenDB())
            return false;

        try
        {
            employee = EmployeeModel.FindById(employeeId);
            Depots = DepotModel.FindAllShort();
            Departments = EmployeeModel.FindAllDepartments();
            TenureHistory = TenureHistoryModel.FindAllByEmployeeId(employeeId);
            IdCards = IdCardModel.FindAllByEmployeeId(employeeId);
            Qualifications = QualificationModel.FindAllByEmployeeId(employeeId);
            TheoryBookings = TheoryBookingModel.FindAllByEmployeeId(employeeId);
            MedicalRecords = MedicalRecordModel.FindAllByEmployeeId(employeeId);
            EyesightChecks = EyesightCheckModel.FindAllByEmployeeId(employeeId);
        }
        catch
        {
            DbConnector.CloseDB();
            return false;
        }

        DbConnector.CloseDB();
        return true;
    }
    public void Refresh(object parameter)
    {
        if (!LoadEmployee(employee.EmployeeId))
            MessageBox.Show("An error has occurred");

        NotifyPropertyChanged(string.Empty);
    }
smally
  • 453
  • 1
  • 4
  • 14

1 Answers1

0

Creating or referencing a view or a window from a view model violates and effectively breaks the MVVM design pattern.

You should inject your MainWindowViewModel with an implementation of an IWindowService that takes care of creating the window:

public class WindowService : IWindowService
{
    public void ShowDialog(EmployeeViewModel viewModel, EmployeeModel model)
    {
        if (LoadEmployee(model.employeeId))
        {
            Window employeeWindow;
            if (model.UserType == "User")
                employeeWindow = new EmployeeUserDetailsView();
            else // if Admin
                employeeWindow = new EmployeeAdminDetailsView();

            employeeWindow.DataContext = viewModel;
            employeeWindow.ShowDialog();
        }
        else
        {
            // Open error message
            MessageBox.Show("Error loading employee occurred");
        }
    }
}

MainWindowViewModel:

private readonly IWindowService _windowService;

public MainWindowViewModel(IWindowService windowService)
{
    _windowService = windowService;
}

public void OpenSelectedEmployee(object employee)
{
    if (employee == null)
        return;

    StaffListModel model = (StaffListModel)employee;

    EmployeeViewModel vm = new EmployeeViewModel(model.EmployeeId);

    _windowService.ShowDialog(vm, model);
}

See my answer here for another example.

mm8
  • 163,881
  • 10
  • 57
  • 88
  • Thanks. I've changed it so my view model constructor takes the employeeId and the user type is fetched from a public method in the view model. I'm not seeing any reason to create the interface though, why not just use the WindowService class? – smally Feb 16 '21 at 21:45
  • @smally: Because it has a strong dependency on the `System.Windows.Window` type and cannot be mocked in unit tests. – mm8 Feb 16 '21 at 22:07
  • So if I were to show a new window from EmployeeViewModel's view, would I need to pass _windowService into EmployeeViewModel's constructor. Or should it be a new one like `EmployeeViewModel vm = new EmployeeViewModel(new WindowService(), model.EmployeeId)` – smally Feb 18 '21 at 22:48
  • Yes, you should pass an implementation of the interface to the view model. – mm8 Feb 19 '21 at 14:34
  • so for the rest of my project, when opening new windows I'm wanting to just open the window or an error message box if the data loading fails. Would doing it this way be a good approach and MVVM compliant? https://pastebin.com/TarUpdac – smally Feb 19 '21 at 23:04
  • Please don't ask additional questions in the comments field. But yes, you should inject all view models from where you intend to open a window with your service interface. – mm8 Feb 23 '21 at 20:50