Adding model into another view model's collection

Question

The Setup

So I'm working on a project in which there exists a MainViewModel class. This MainViewModel contains a list of Soldiers through an observable collection. I have a button in the MainView which displays a new AddSoldierView. The AddSoldierView is binding to elements from the AddSoldierViewModel. The AddSoldierView is basically a form where the user inputs all of the data for that soldier.

The Problem

Now that I have the Soldier's information on the AddSoldierViewModel, I want to be able to add that back into the ObservableCollection of the MainViewModel. I have binded a command to the button (Add Soldier) on the AddSoldierView, but I'm not sure how to get that information back into the MainViewModel.

What I've tried

I've already setting up an event handler on the AddSoldierViewModel in which the SoldierModel is passed as an EventArg. But I can't get the event itself to trigger.

Any suggestions? I've been trying to stay true to the MVVM spirit, but there are still some kinks I'm trying to sort out. Let me know if you want to see some code snippets, UML diagrams, or whatever.

AddSoldierViewModel.cs

public class AddSoldierViewModel : ViewModelBase
{
    public event EventHandler<AddSoldierEventArgs> AddSoldierClicked;

    public ICommand AddSoldierCommand;

    private Soldier _soldier;

    public Soldier Soldier
    {
        get => _soldier;
        set
        {
            _soldier = value;
            RaisePropertyChanged(nameof(Soldier));
        }
    }

    public AddSoldierViewModel() 
    {
        AddSoldierCommand = new RelayCommand(AddSoldier);
    }

    private void AddSoldier()
    {
        OnAddSoldierClicked(new AddSoldierEventArgs()
        {
           Soldier = Soldier
        });
    }

    protected virtual void OnAddSoldierClicked(AddSoldierEventArgs e)
    {
        var handler = AddSoldierClicked;
        handler?.Invoke(this, e);
    }
}

MainViewModel.cs

public class MainViewModel : ViewModelBase
{
    #region - Private Properties -
    private Team _selectedTeam;
    private Soldier _selectedSoldier;
    #endregion // - Private Properties -

    #region // - Public Properties -
    public ObservableCollection<Soldier> Soldiers { get; set; }
    public ObservableCollection<Team> Teams { get; }
    public Team SelectedTeam
    {
        get => _selectedTeam;
        set
        {
            _selectedTeam = value;
            RaisePropertyChanged(nameof(SelectedTeam));
        }
    }
    public Soldier SelectedSoldier
    {
        get => _selectedSoldier;
        set
        {
            _selectedSoldier = value;
            RaisePropertyChanged(nameof(SelectedSoldier));
        }
    }
    #endregion // - Public Properties -

    #region // - Commands -
    public ICommand DeleteTeamCommand { get; private set; }
    public ICommand AddSoldierDialogCommand { get; private set; }
    #endregion // - Commands -

    #region  - Services -
    public IDialogService AddSoldierDialogService { get; private set; }
    #endregion // - Services -

    #region - Constructors -
    public MainViewModel()
    {
        Soldiers = new ObservableCollection<Soldier>();
        Teams = new ObservableCollection<Team>();

        Soldiers.CollectionChanged += Soldiers_CollectionChanged;
        Teams.CollectionChanged += Teams_CollectionChanged;

        DeleteTeamCommand = new RelayCommand(DeleteTeam);
        AddSoldierDialogCommand = new RelayCommand(AddSoldierDialog);

        AddSoldierDialogService = new AddSoldierDialogService();
    }

    #endregion // - Constructors -

    #region - Methods -
    private void AddSoldierDialog()
    {
        AddSoldierViewModel addSoldierViewModel = new AddSoldierViewModel();
        addSoldierViewModel.AddSoldierClicked += AddSoldierViewModel_AddSoldierClicked;
        AddSoldierDialogService.ShowDialog(addSoldierViewModel);
    }

    private void AddSoldierViewModel_AddSoldierClicked(object sender, AddSoldierEventArgs e)
    {
        Soldiers.Add(new Soldier(e.Soldier));
    }

    private void Teams_CollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
    {
        foreach (var item in e.NewItems)
        {
        }
        foreach (var item in e.OldItems)
        {
        }
        RaisePropertyChanged(nameof(Teams));
    }

    private void Soldiers_CollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
    {
        foreach (var item in e.NewItems)
        {
        }
        foreach (var item in e.OldItems)
        {
        }
        RaisePropertyChanged(nameof(Soldiers));
    }
    #endregion // - Methods -
}

Answer 1

Since the AddSoldierViewModel is for a dialog box, I think you can make things a lot simpler. The only thing that needs to change is the AddSoldierDialog() method:

private void AddSoldierDialog()
{
    AddSoldierViewModel addSoldierViewModel = new AddSoldierViewModel();

    if(AddSoldierDialogService.ShowDialog(addSoldierViewModel) == true)
    {
        Soldiers.Add(addSoldierViewModel.Soldier);
    }
}

The return value of ShowDialog() should be bool?, where true means the user closed the dialog in a good state. false would be closing the dialog in a bad state and null would be closing the dialog from the window itself. Once you know the user intended to add that soldier, you can add it to your list directly.

This avoids complicated event routes and simplifies debugging.

---

Of course, if the use case of the "Add Soldier Dialog" were to stay open and allow the user to add multiple new soldiers before closing, then your event would be necessary. Just make sure to unregister it when you don't need it any more.

Last comment would be if you do choose to keep the event, you handle copying the Soldier instance before you send it. That way you don't have lingering bugs because somewhere someone thought the soldier in the event was unique.

In that case your click handler would look something like this:

private void RaiseAddSoldierClicked()
{
    AddSoldierClicked?.Invoke(this, new AddSoldierEventArgs
    {
        Soldier = new Soldier(Soldier)
    });
}

Handling that on the event source instead of all the listeners will help reduce unexpected bugs.

Answer 2

So I've done some more messing around last night and I think I may have found a good solution for MVVM. I revisited the idea of the Event and added an EventHandler AddSoldierClick to the AddSoldierVM. I also created a custom EventArgs class to hold the selected Soldier.

When I instantiate a AddSoldierVM on my MainVM, I subscribe to the AddSoldierClick event and add the EventArg's Soldier object to my ObservableCollection in the MainVM.

After that, I just RaisePropertyChanged(nameof(Soldiers)) in the Soldiers_CollectionChanged() event.

---

I'll go and post code snippets when I get home. But I decided to share this answer with you guys. Are there any other ways to go about it? I think this one is pretty clean (just gotta watch out for the potential memory leaks in subscribing to the event and forgetting to unsubscribe when the MainVM is done).