Code-Review: MVVM mit mehr als einem VieModel

Hallo,
nach etlichen Versuchen, Tutorials und der oft nicht ganz einfachen MSDN wähnte ich mich bereits am Ziel, mit dem simpelst vorstellbaren Ziel: Ein Hauptfenster, mit zwei verschiedenen GUI-Bereichen, jedes mit eigener View (usercontrol) und eigenem VIewModel. Eines enthält eine Textbox, das andere einen Textblock - was ich in die Textbox eingebe, soll im Textblock zu sehen sein.
Mein Code verwendet keinen Code behind. Ich habe versucht nach meinem Kenntnis- und Wissensstand (gefühlt immer noch Anfänger...) MVVM umzusetzen.

[WPF app, .NET 6]

Ich weiß, meine Frage ist vermutlich ein alter Hund, aber es würde mir sehr helfen, wenn mir jemand aufzeigen könnte an welchem Punkt und warum mein Code sich nicht so verhält, wie von mir beabsichtigt. Ich werde außerdem am Ende kurz meine Gedanken zu einzelnen Codesegmenten darlegen, in der Hoffnung, dass mein Fehler evtl. auf Missverständnissen oder Fehlinterpretationen beruht.

Ohne weitere Worte - hier der Code

1. MainWindowView

<Window x:Class="LearnViewModelsAndViews.Views.MainWindowView"
        [...]
    <Window.DataContext>
        <viewModels:MainWindowViewModel/>
    </Window.DataContext>
    
    <Grid>
        <Grid Margin="10">
            <Grid.RowDefinitions>
                <RowDefinition/>
                <RowDefinition/>
            </Grid.RowDefinitions>

            <Grid Grid.Row="0">
                <ContentControl>
                    <viewModels:RedViewModel/>
                </ContentControl>
            </Grid>

            <Grid Grid.Row="1">
                <ContentControl>
                    <viewModels:BlueViewModel/>
                </ContentControl>
            </Grid>
        </Grid>
    </Grid>
</Window>

Und das zugehörige MainWindowViewModel

 public class MainWindowViewModel: NotifyPropertyChangedBase
    {
        private RedViewModel _redVM;
        public RedViewModel RedVM
        {
            get { return _redVM; }
            set { OnPropertyChanged(ref _redVM, value); }
        }

        private BlueViewModel _blueVM;
        public BlueViewModel BlueVM
        {
            get { return _blueVM; }
            set { OnPropertyChanged(ref _blueVM, value); }
        }


        public MainWindowViewModel()
        {
            RedVM = new RedViewModel();
            BlueVM = new BlueViewModel();

            SetEvents();
        }

        private void SetEvents()
        {
            RedVM.OnInputTextChanged += BlueVM.HandleInputTextChanged;
        }
    }
}

XAML von RedView

<UserControl x:Class="LearnViewModelsAndViews.Views.RedView"
          [...]
    <Grid Background="LightCoral">       
        <Grid>
            <StackPanel VerticalAlignment="Center" HorizontalAlignment="Center">
                <TextBox Width="220" Height="60" Text="{Binding InputText, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}" FontSize="18"/>
            </StackPanel>
        </Grid>            
    </Grid>
</UserControl>

Und das RedViewModel

public class RedViewModel: NotifyPropertyChangedBase
    {
		public event EventHandler OnInputTextChanged;

		private string _inputText;
		public string InputText
		{
			get { return _inputText; }
			set 
			{ 
				OnPropertyChanged(ref _inputText, value); 
				OnInputTextChanged?.Invoke(value, EventArgs.Empty);
			}
		}

		public RedViewModel()
		{
		}
	}

Hier die BlueView.xaml

<UserControl x:Class="LearnViewModelsAndViews.Views.BlueView"
             [...]
    <Grid Background="LightBlue">
        <Grid>
            <StackPanel VerticalAlignment="Center" HorizontalAlignment="Center">
                <TextBlock Width="220" Height="60" FontSize="18" Foreground="Red" Text="{Binding DisplayText, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"/>                
            </StackPanel>                        
        </Grid>
            
    </Grid>
</UserControl>

Und das zugehörige ViewModel

 public class BlueViewModel: NotifyPropertyChangedBase
    {
		private string _displayText;
		public string DisplayText
		{
			get { return _displayText; }
			set { OnPropertyChanged(ref _displayText, value); }
		}
		
		public BlueViewModel()
		{
		}

		public void HandleInputTextChanged(object sender, EventArgs e)
		{
			DisplayText = sender.ToString();
		}

	}

Last but not least meine App.xaml

<Application x:Class="LearnViewModelsAndViews.App"
           [...]
             StartupUri="Views/MainWindowView.xaml">
    <Application.Resources>

        <DataTemplate DataType="{x:Type viewModels:RedViewModel}">
            <views:RedView/>
        </DataTemplate>

        <DataTemplate DataType="{x:Type viewModels:BlueViewModel}">
            <views:BlueView/>
        </DataTemplate>

    </Application.Resources>
</Application>

OK: mein Projekt wird wie vorgesehen erstellt und ich bekomme ein Projektfenster angezeigt mit meinem RedView im oberen Bereich und meiner BlueView im unteren Bereich. Soweit alles paletti.
Gebe ich nun Text in die Textbox der RedView ein, wird das OnPropertyChanged() zwar gefeuert, der Event darunter (OnInputTextChanged) ist jedoch null.

Meine Fragen soweit: wo ist der Denkfehler? Was muss ich ändern?

Die Tatsache, dass der event null ist, interpretiere ich so, dass mein BlueVM nicht das ViewModel ist, das ich anzusprechen beabsichtige. Hier kommt nun (mit ziemlicher Sicherheit) Halbwissen ins Spiel:

• mit den DataTemplates in der App.xaml sage ich doch nichts anderes als, dass wenn das ViewModel vom Typ RedViewModel ist, dann gehört dazu die RedView, bzw. umgekehrt. Sehr vereinfacht und vermutlich ungelenk formuliert. Aber auch völlig falsch?
• ich instanziiere ein RedViewModel (RedVM) und ein BlueViewModel (BlueVM) auf meinem MainViewModel. Da ich die Werteveränderung von InputText per event weiterleiten möchte, platziere ich hier meine Subscription ( RedVM.OnInputTextChanged += BlueVM.HandleInputTextChanged; ) Allerdings werden diese ViewModels vermutlich gar nicht angesprochen: irgendwo "hinter den Kulissen" erstelle ich wohl (unbeabsichtigt) eine andere Instanz von RedViewModel und BlueViewModel - und dann ist klar, dass der Compiler nicht wissen kann welche Instanz jetzt implizit gemeint ist.

Wie gesagt: für konkrete Hinweise bin ich euch sehr dankbar. Ich möchte mir nur ungern ein neues Hobby suchen 😉

Gruß
Vorph

mit den DataTemplates in der App.xaml sage ich doch nichts anderes als, dass wenn das ViewModel vom Typ RedViewModel ist, dann gehört dazu die RedView, bzw. umgekehrt

Korrekt - bis auf das "umgekehrt" 😉
Damit definierst Du global einen DataTemplate für den Datentyp, der verwendet wird, wenn kein DataTemplate angegeben wurde.
Eine umgekehrte Verbindung View->ViewModel gibt's dagegen nicht.

Würde ich so aber nicht machen.
Hat seine Daseinsberechtigung, aber wenn man das nicht explizit braucht, würde ich das Control an der Stelle einfach direkt einbauen, ohne ContentControl und DataTemplate.
Oder Du definierst das DataTemplate direkt dort, wo Du es brauchst und nicht global, in größeren Projekten wird das sonst schnell chaotisch.

Ich hab das bei meinem letzten WPF-Projekt für die Navigation genutzt.
Die ViewModels haben mit ViewModels navigiert und global waren DataTemplates definiert, damit WPF die View dazu findet.
Ich hatte also irgendwo ein ContentControl und ein Binding auf "CurrentViewModel" und das ContentControl hat sich die View gesucht.
Aber so spontan fällt mir neben einer Navigation kein Grund ein, warum man das brauchen sollte.

Allerdings werden diese ViewModels vermutlich gar nicht angesprochen: irgendwo "hinter den Kulissen" erstelle ich wohl (unbeabsichtigt) eine andere Instanz von RedViewModel und BlueViewModel

Exakt das ist dein Problem.
Das ist aber nichts hinter den Kulissen, sondern im Code "right in your face" 😉
Entscheidend sind in der MainWindowView die Zeilen 16 und 22, da instanziierst Du die zwei ViewModels.

Richtig wäre:

<ContentControl Content="{Binding RedVM}" />

Oder ohne DataTemplate:

<views:RedView DataContext="{Binding RedVM}" />

Ansonsten noch ein paar Anmerkungen:

Alles readonly, was readonly geht (oder wenigstens private)
Oder in deinem Fall: Mach die ViewModel-Properties im MainViewModel readonly
Die ändern sich nicht, dann müssen sie sich auch nicht ändern können.
Das spart Code und in einem Team kann niemand auf die Idee kommen, da doch mal was zu ändern - naja, zumindest nicht so leicht.

Ich halte das für mich immer so:
Jede Klasse, die erlaubt, dass ein Wert geändert werden kann, die kann auch jederzeit damit umgehen.
Umgekehrt alles, was nicht jederzeit damit umgehen kann, oder wo ich das nicht brauche, das ist readonly.
Und alles, was sich nur von "innen" ändern darf, wird private gemacht.

Sei verdammt vorsichtig mit Events.
Events können Vorteile haben und sind sicher nützlich, aber wenn Du nicht aufpasst, können sie die pure Hölle sein 😉
Hier hast Du zwei Komponenten, die ja eigentlich voneinander unabhängig sein sollen, die im Code auch unabhängig sind, zur Laufzeit aber nicht.
Hier ist das Nebensache, aber in größeren Projekten geht sowas schnell unter und sorgt früher oder später für Kopfschmerzen.

Besser wäre, wenn Du etwas anderes hast, was den Fluss steuert, also eine Art Backend oder Business-Layer oder in der MVVM-Terminologie das Model.
Das eine ViewModel schreibt den neuen Wert ins Model.
Das andere ViewModel liest den neuen Wert.
Das Model kümmert sich dann darum, dass das andere ViewModel informiert wird, da kannst Du wieder ein Event nutzen.
Wenn nun noch ein drittes/viertes/fünftes ViewModel dazu kommt, ändert sich an der Komplexität nichts, die fünf ViewModels überwachen sich nicht gegenseitig, sondern nur das Model und im Model kannst Du alles steuern steuern.
Oder Du steckst eine Art "Zwischen-ViewModel" dazwischen, was Änderungen entgegen nimmt, über neue Werte informiert und das Laden/Speichern zum "echten" Model übernimmt.

Wichtig dabei ist, dass die ViewModels nicht quer­beet auf Events der Anderen horchen, sondern dass Du einen klar gesteuerten Fluss hast.
Das eine Event ist dann dieser klar gesteuerte Fluss, oder Du nutzt was anderes, z.B. das Event Aggregator Pattern, aber das geht dann über MVVM hinaus.

Ach und beachte natürlich, dass Events gerne für Memory Leaks verantwortlich sind.

Der "sender" bei events ist immer der, der das Event geworfen hat.
Über den sender-Parameter übergibst Du (fast) immer nur this, sonst nichts.
Wenn Du Daten übergeben willst, dann verwende eine eigene EventArgs-Ableitung und das passende EventHandler<MyEventArgs>

Binding-Mode/-Trigger nur, wenn Du es brauchst
Meistens ist der Binding-Mode-Default ganz gut gewählt, wahrscheinlich brauchst Du es in beiden Fällen nicht.
Und der UpdateSourceTrigger bei dem TextBlock ist unsinnig.

Hallo Palladin007,

vielen Dank für deine ausführliche Antwort! Deine Mühe (zu nachtschlafener Zeit) hat sich absolut gelohnt - heute morgen fallen mir gleich reihenweise die Schuppen von den Augen! Alles, was du unter "ein paar Anmerkungen" geschrieben hast, hilft mir weit über die eigentliche Fragestellung hinaus weiter - besonders hinsichtlich der Planung für kommende Projekte.

Exakt das ist dein Problem.
Das ist aber nichts hinter den Kulissen, sondern im Code "right in your face"

Ich würde jetzt gerne sagen "Ich wusste es!", aber ich wusste ehrlich nicht genau wo 😉 Und das beruhte auf der einen "kleinen" Fehlannahme (die du glücklicherweise auch herausgestellt hast), dass

dann gehört dazu die RedView, bzw. umgekehrt

dies zutreffend ist. Tatsächlich hatte ich

                 <ContentControl>
                    <viewModels:RedViewModel/>
                </ContentControl>

auch schon gegen

                 <ContentControl>
                    <views:RedView/>
                </ContentControl>

getauscht. In beiden Fällen wird die jeweilige View korrekt im Designer angezeigt. Nachträglich möchte ich mich an dieser Stelle selbst ohrfeigen, denn hätte ich richtig geschaltet, hätte ich bemerkt, dass nun der OnPropertyChanged nicht mehr ausgelöst wird! Damit hätte ich eine erste Spur gehabt, dass das was ich sehe und das, was der Code tut nicht korreliert. Und dann hätte mich auch die Tatsache stutzig machen müssen, dass diese "kleine" Änderung plötzlich dazu führt, dass View und ViewModel sich plötzlich nicht mehr finden --> ergo muss das mit dem "umgekehrt" falsch sein und es ist an der Stelle einleuchtend, dass meine Vermutung (ein neues VM würde instanziiert) wahrscheinlich an genau der Stelle passiert 😭

Tatsächlich waren die Tutorials mehrere mit dem Thema ViewModel-Navigation: und da funktioniert es ja auch prächtig. Meine Rückschlüsse waren die Falschen! Denn beim Binding auf Properties läuft es ja auch nach dem Schema {Binding <PropertyName>} - und Halleluja, da habe ich zwei ViewModel-Properties und vermassele das Binding 1x1!

Wie gesagt: 1000-Dank für deine Antwort, das hat mir sehr weitergeholfen. Manchmal muss man eben einfach seine Annahmen kritisch hinterfragen^^
Übrigens:

Besser wäre, wenn Du etwas anderes hast, was den Fluss steuert, also eine Art Backend oder Business-Layer oder in der MVVM-Terminologie das Model.

Genau so hatte ich es vor, daher eine kleine Anschlussfrage: ich hatte vor diese Klasse "Mediator" oder meinetwegen auch "ViewModelMediator" zu nennen. Nun sehe ich gerade, da gibt's das Mediatorpattern und ich bin noch nicht ganz durch. Also a) entweder ist das genau das worüber wir hier reden oder b) es ist doch waa anderes. Und nein, ich bin nicht in einem Team und werde wohl nie in die Verlegenheit kommen, einem Team anzugehören - bin wirklich nur Hobbycoder. Aber: ist der Name "Mediator" i. d. Fall geschickt gewählt oder gibt das Verwirrung bezüglich Mediator-Pattern?

Viele Grüße
Vorph

Das Mediator Pattern gibt es, aber ich denke, das schießt hier weit über's Ziel hinaus.
Natürlich kannst Du das bei einer größeren Anwendung so machen, bei kleinen Anwendungen reicht aber auch ein schlichtes Event.
Ich kenne das Mediator Pattern auch eher in umgekehrter Richtung, also dass die ViewModels über den Mediator mit dem Backend kommunizieren.
Außerdem behandelt das Mediator Pattern nicht nur das Informieren über Ereignisse, sondern auch, wie diese Ereignisse überhaupt erst angestoßen werden.

Mit dem Mediator Pattern wäre es also:*VideModel sendet SetTextRequest zum Backend. *Im Backend gibt's einen SetTextRequestHandler, der den Wert speichert. *Im Backend wird eine TextChangedNotification gesendet. *Im Frontend gibts viele TextChangedNotificationHandler, die den Wert in der UI aktualisieren.

Aber wie gesagt: Das halte ich für overkill.
Wenn Werte über mehrere ViewModels benötigt werden und Änderungen erst im RAM vorgehalten werden sollen, würde ich eine Art DataViewModel aufbauen.
Dieses DataViewModel wird beim ersten Laden der Daten aufgebaut, jedes andere ViewModel arbeitet damit und das DataViewModel bietet Event(s), um über Änderungen zu informieren.
Und zum Schluss gibt's im DataViewModel eine Methode/einen Command zum Speichern des aktuellen Standes oder sowas wie Undo/Redo-Funktionen.
Das DataViewModel kommuniziert dann erst mit dem Backend.