# Caliburn.Micro nested ViewModels

I am currently developing a small tool intended to help me keep track of the myriad of characters in my Stories.

The tool does the following:

• Load the characters which are currently stored as json on the disk and stores them in a list, which is presented in the Shell via a ListBox.
• If the user then opens a character the Shell, which is a Conductor<Screen>.Collection.OneActive, opens a new CharacterViewModel, that derives from Screen.
• The Character gets the Character that is going to be opened via the IEventAggregator message system.
• The CharacterViewModel furthermore has various properties which are sub ViewModels which bind to various sub Views.

And here is my Problem: Currently I initialize the sub ViewModels manually when the ChracterViewModel is initialized. But this sounds fishy to me and I am pretty sure there is a better way to do this, but I cannot see how I should do it.

Here is the code of the CharacterViewModel:

/// <summary>ViewModel for the character view.</summary>
public class CharacterViewModel : Screen, IHandle<DataMessage<ICharacterTagsService>>
{
// --------------------------------------------------------------------------------------------------------------------
// Fields
// -------------------------------------------------------------------------------------------------------------------

/// <summary>The event aggregator.</summary>

/// <summary>The character tags service.</summary>
private ICharacterTagsService characterTagsService;

// --------------------------------------------------------------------------------------------------------------------
// Constructors & Destructors
// -------------------------------------------------------------------------------------------------------------------

/// <summary>Initializes a new instance of the <see cref="CharacterViewModel"/> class.</summary>
public CharacterViewModel()
{
if (Execute.InDesignMode)
{
this.CharacterGeneralViewModel = new CharacterGeneralViewModel();

}
}

/// <summary>Initializes a new instance of the <see cref="CharacterViewModel"/> class.</summary>
/// <param name="eventAggregator">The event aggregator.</param>
[ImportingConstructor]
public CharacterViewModel(IEventAggregator eventAggregator)
: this()
{
this.eventAggregator = eventAggregator;
this.eventAggregator.Subscribe(this);
}

// --------------------------------------------------------------------------------------------------------------------
// Properties
// -------------------------------------------------------------------------------------------------------------------

/// <summary>Gets or sets the character.</summary>
public Character Character { get; set; }

/// <summary>Gets or sets the character general view model.</summary>
public CharacterGeneralViewModel CharacterGeneralViewModel { get; set; }

/// <summary>Gets or sets the character metadata view model.</summary>

/// <summary>Gets or sets the character characteristics view model.</summary>
public CharacterApperanceViewModel CharacterCharacteristicsViewModel { get; set; }

/// <summary>Gets or sets the character family view model.</summary>
public CharacterFamilyViewModel CharacterFamilyViewModel { get; set; }

// --------------------------------------------------------------------------------------------------------------------
// Methods
// -------------------------------------------------------------------------------------------------------------------

/// <summary>Saves a character to the file system as a json file.</summary>
public void SaveCharacter()
{
ICharacterSaveService saveService = new JsonCharacterSaveService(Constants.CharacterSavePathMyDocuments);

saveService.SaveCharacter(this.Character);

this.characterTagsService.SaveTags();
}

/// <summary>Called when initializing.</summary>
protected override void OnInitialize()
{
this.CharacterGeneralViewModel = new CharacterGeneralViewModel(this.eventAggregator);
this.CharacterCharacteristicsViewModel = new CharacterApperanceViewModel(this.eventAggregator, this.Character);
this.CharacterFamilyViewModel = new CharacterFamilyViewModel(this.eventAggregator);

{
Data = this.Character
});

base.OnInitialize();
}

/// <summary>
/// Handles the message.
/// </summary>
/// <param name="message">The message.</param>
public void Handle(DataMessage<ICharacterTagsService> message)
{
this.characterTagsService = message.Data;
}
}


For Completion Sake I also give you one of the sub ViewModels. The others a of no importance because they are structured the same way, just perform different tasks.

/// <summary>The character metadata view model.</summary>
{
/// <summary>The event aggregator.</summary>

/// <summary>Initializes a new instance of the <see cref="CharacterMetadataViewModel"/> class.</summary>
{
if (Execute.InDesignMode)
{
}
}

/// <summary>Initializes a new instance of the <see cref="CharacterMetadataViewModel"/> class.</summary>
/// <param name="eventAggregator">The event aggregator.</param>
/// <param name="character">The character.</param>
{
this.Character = character;

this.eventAggregator = eventAggregator;
this.eventAggregator.Subscribe(this);
}

/// <summary>Gets or sets the character.</summary>
public Character Character { get; set; }

/// <summary>
/// Gets or sets the characters tags.
/// </summary>
public string Tags
{
get
{
}

set
{
char[] delimiters = { ',', ';', ' ' };

List<string> tags = value.Split(delimiters, StringSplitOptions.RemoveEmptyEntries).ToList();

this.NotifyOfPropertyChange(() => this.Tags);
}
}
}


I already read in on Screens, Conductors and Composition, IResult and Coroutines and skimmed the rest of the Documentation, but somehow I cannot find what I am looking for.

I should mention the code I have works just fine. I'm just not satisfied with it, since I think I am not understanding the concept of MVVM quite right and therefore make faulty code.

I need to comment on ...your comments. Really. NONE of the comments you have here are useful.

None of them.

This is rather unusual:

/// <summary>ViewModel for the character view.</summary>


Normally XML comments looks like this:

/// <summary>
/// ViewModel for the character view.
/// </summary>


That's also how the IDE sets them up automatically for you when you type /// (assuming Visual Studio), so having them on 1 line seems like you're fighting your IDE. Embrace it!

The purpose of XML comments is to provide documentation, specifically for the public API. This:

/// <summary>The event aggregator.</summary>


...is overkill useless clutter. If you need a comment that says "The event aggregator" for a private field called eventAggregator of type IEventAggregator, ... same here:

/// <summary>The character tags service.</summary>
private ICharacterTagsService characterTagsService;


This is rather annoying:

// --------------------------------------------------------------------------------------------------------------------
// Fields
// -------------------------------------------------------------------------------------------------------------------


Kill it, with fire. Burn. The maintainer that cannot tell a field from a property shouldn't be maintaining your code. The only thing worse you could have done, is wrap them in a #region Fields.

## This

I find the this qualifier adds to the clutter, although here it's needed:

public CharacterViewModel(IEventAggregator eventAggregator)
: this()
{
this.eventAggregator = eventAggregator;
this.eventAggregator.Subscribe(this);
}


...but only because eventAggregator isn't called _eventAggregator. All other instances of this can be removed.

That said...

### Constructor chaining

You have it reversed: the parameterless/default constructor should be calling the parameterized constructor, not the other way around:

public CharacterViewModel()
: this(null)
{
// ...
}


Why are you injecting an IEventAggregator (which is good), but instantiating CharacterGeneralViewModel and CharacterMetadataViewModel? They could be constructor-injected as well.

• Regarding the usage of this. Actually Stylecop and ReSharper tell me to use this. Since I personally feel it makes the code more readable and according to [stackoverflow.com/questions/449324/… (this) SO Question it makes no difference I will keep it. As for contructor chaining: I always thought you chained the constructor from more to less. Is there any reason to it the other way around? – Ruhrpottpatriot Sep 4 '14 at 7:15
• To this or not to this is really more of a preference, I should have pointed out that I like your consistency here. As for constructors, chaining so as to always end up calling that with the most parameters allows you to move more code to a single constructor, and makes the defaults more obvious (you have an implicit null default with the parameterless constructor); it's all about maintainability ;) – Mathieu Guindon Sep 4 '14 at 13:26