MVP Winforms : verstehen und lernen, ist es so richtig?

Hallo,

ich beschäftige mich gerade mit den Pattern, und mache gerade meine ersten geh versuche mit dem MCP Pattern.

Wäre nett wenn mal wer darüber schaut ob das dem MVP entspricht?
Als Anfänger tu ich mich noch gerade sehr hart zu verstehen wieso ich mich so jetzt so verbiegen muss bzw wo die Vorteile und Nachteile liegen

	public class MainForm_Presenter
	{
		private IModel _model;
		private IMainForm _view;

		public MainForm_Presenter(IModel model, IMainForm view)
		{
			this._model = model;
			this._view = view;
			this._view.LoadData(_model.GetListOfPLCs());
		}

		public void UpdateDatabase()
		{
			this._model.InsertPLC(_view.addItem);
			this._view.LoadData(_model.GetListOfPLCs());
		}
	}

	public interface IMainForm
	{
		void LoadData(IList<PLCTyp> data);
		PLCTyp addItem { get; }
	}

	public partial class MainForm : Form, IMainForm
    {
		private MainForm_Presenter _presenter = null;

		public MainForm()
        {
            InitializeComponent();
			Model model = new Model();
			_presenter = new MainForm_Presenter(model, this);
            ListView_Init();
        }

        private void ListView_Init()
        {
            listView1.Columns.Add("id");
            listView1.Columns.Add("name");
            listView1.Columns.Add("ip");
            listView1.Columns.Add("connection");
            listView1.Columns.Add("cycle[ms]");

            // Create the ContextMenuStrip.
            var docMenu = new ContextMenuStrip();
            //Create some menu items.
            ToolStripMenuItem delete = new ToolStripMenuItem();
            delete.Text = "delete";
			//delete.Click += deletePLCToolStripMenuItem_Click;

            ToolStripMenuItem showDBEditor = new ToolStripMenuItem();
			showDBEditor.Text = "db editor";
			//showDBEditor.Click += showDBPanel;

            //Add the menu items to the menu.
            docMenu.Items.Add(showDBEditor);
            docMenu.Items.Add(delete);

            listView1.ContextMenuStrip = docMenu;
        }

		public void LoadData(IList<PLCTyp> data)
		{
			listView1.BeginUpdate();
			listView1.Items.Clear();
			foreach (PLCTyp row in data)
			{
				ListViewItem item = new ListViewItem();
				item.SubItems.Add(row.name);
				item.SubItems.Add(Convert.ToString(row.ip));
				item.SubItems.Add(Convert.ToString(row.target));
				item.SubItems.Add(Convert.ToString(row.cycle));
				this.listView1.Items.Add(item);
			}
			listView1.EndUpdate();
		}

		private void btn_Add_Click(object sender, EventArgs e)
		{
			_presenter.UpdateDatabase();
		}

		public PLCTyp addItem
		{
			get
			{
				return
			  new PLCTyp()
			  {
				  name = textBox_name.Text,
				  cycle = Convert.ToInt16(textBox_cylce.Text),
				  ip = textBox_ip.Text,
				  target = comboBox_typ.Text,
			  };
			}
		}
	}[CSHARP]

public interface IModel
	{
		void ClearDatabase();
		void CreatDatabase();
		List<PLCTyp> GetListOfPLCs();
		void deletePLC(PLCTyp plc);
		void InsertPLC(PLCTyp pLCTyp);
		void InsertDB(PLCTyp pLCTyp, DBTyp dBTyp);
		void InsertVar(VarTyp varTyp);
		PLCTyp GetPlcByID(int id);
		List<DBTyp> GetDBByPlc(PLCTyp plc);
		List<VarTyp> GetVarByDB(VarTyp var);
	}

	public class Model : IModel
    {
		public Model() {}

		#region MANAGMENT
		public void ClearDatabase()
		{
			using (var db = new DBContext())
			{
				db.ClearDatabase();
			}
		}

		public void CreatDatabase()
		{
			using (var db = new DBContext())
			{
				db.CreateDatabase();
			}
		}
		#endregion

		#region DELETE
		public void deletePLC(PLCTyp plc)
		{
			try
			{
				using (var db = new DBContext())
				{
					db.PlcSets.Remove(plc);
					db.SaveChanges();
				}
			}
			catch (Exception ex)
			{
				Console.WriteLine(ex.Message);
			}
		}
		#endregion
		
		#region INSERT
		public void InsertPLC( PLCTyp pLCTyp)
        {
         try
            {
                using (var db = new DBContext())
                {
                    db.PlcSets.Add(pLCTyp);
                    db.SaveChanges();
                }
            }
            catch (Exception ex)
            {
				Console.WriteLine(ex.Message);
			}
		}

		public void InsertDB( PLCTyp pLCTyp, DBTyp dBTyp)
        {
            try
            {
                using (var db = new DBContext())
                {
					var plc = db.PlcSets.Where(p => p == pLCTyp)
						.Include(p => p.DbSets)
										.Single();
					plc.DbSets.Add(dBTyp);
                    db.SaveChanges();
                }
            }
            catch (Exception ex)
            {
				Console.WriteLine(ex.Message);
			}
		}

		public void InsertVar(VarTyp varTyp)
        {
            try
            {
                using (var db = new DBContext())
                {
                    db.VarSets.Add(varTyp);
                    db.SaveChanges();
                }
            }
            catch (Exception ex)
            {
				Console.WriteLine(ex.Message);
			}
		}
		#endregion INSERT

		#region GET
		public List<PLCTyp> GetListOfPLCs()
		{
			try
			{
				using (var db = new DBContext())
				{
					var plcSets = db.PlcSets
						.ToList();

					return plcSets;
				}
			}
			catch (Exception ex)
			{
				Console.WriteLine(ex.Message);
			}
			return null;
		}

		public PLCTyp GetPlcByID(int id)
		{
			try
			{
				using (var db = new DBContext())
				{
					var plc = db.PlcSets
						.Where(b => b.PLCTypId == id)
						.Single();

					return plc;
				}
			}
			catch (Exception ex)
			{
				Console.WriteLine(ex.Message);
			}
			return null;
		}

		public List<DBTyp> GetDBByPlc(PLCTyp plc)
        {
            try
            {
                using (var db = new DBContext())
                {
					var dbs = db.DbSets
						.Where(b => b.PLCTypId == plc.PLCTypId)
						.ToList();

					return dbs;
                }
            }
            catch (Exception ex)
            {
				Console.WriteLine(ex.Message);
			}
			return null;
        }

		public List<VarTyp> GetVarByDB(VarTyp var)
		{
			try
			{
				using (var db = new DBContext())
				{
					var vars = db.VarSets
						.Where(b => b.DBTypId == var.DBTypId)
						.ToList();

					return vars;

				}
			}
			catch (Exception ex)
			{
				Console.WriteLine(ex.Message);
			}
			return null;
		}
		#endregion
	}

Gruß

Grundlegende Dinge: =)

• UI gehört nicht in den Data Layer - dazu gehört auch Console.WriteLine.
• Datenbank-Kontext nicht in dieser Form isoliert erstellen. Schau Dir den Repository Pattern mit UnitOfWork an.
• Exceptions nicht blind im DAL unterdrücken. null zurück geben ist kein valides Exception Handling und gehört zur absoluten Bad Practise
• "Model" als Klasse für Data Access zu bezeichnen, ist nicht der Sinn von Klassennamen.
  Hier erneut der Hinweis auf den Repository Pattern und ein ordentliches Naming.
  Man sollte anhand der Klassenbezeichnung sofort erkennen, was Sache ist.
• Dass Du direkt in der UI auf Datenbank-Funktionalitäten zurück greifst, verletzt prinzipiell [Artikel] Drei-Schichten-Architektur und damit auch den MVP

Die Vorteile von solchen Pattern im Allgemeinen sind:

• Saubere Software Architektur: Trennung von Logik und UI
• Testbarer Code ( [Artikel] Unit-Tests: Einführung in das Unit-Testing mit VisualStudio )

Das sind zwei enorm (mit die wichtigsten) wichtige Punkte in der Software Entwicklung. 😉

Erst mal danke für die Antwort.

• UI gehört nicht in den Data Layer - dazu gehört auch Console.WriteLine.

Dafür gibts später eine Console im View, ist also erst mal nur fürs mitbekommen so gelöst.

• Dass Du direkt in der UI auf Datenbank-Funktionalitäten zurück greifst, verletzt prinzipiell [Artikel] Drei-Schichten-Architektur und damit auch den MVP

Da bin ich gerade nicht auf höhe, wo denn? Ich kommuniziere doch nur über das Interface?

Ich stehe gerade noch vor einem anderen Problem....
Ich hab in meinem MainView 2 SplitContainer jetzt frage ich mich wenn ich das MVP Pattern einhalten will wo erzeuge ich den neuen View( mit Presenter ).
Wenn ich es richtig verstanden habe wird beim MVP immer alles erst im View ausgelöst also gehört es dahin?

Und jetzt noch eine Frage zum Model, sollten die anderen Panels auch auf das selbe Model zugreifen?

Gruß

Du übersiehst, dass mit drei Schichten-Trennung die übergeordnete Architektur gemeint ist. Das MVP-Pattern die Art, wie man die Präsentationsschicht (die UI) umsetzen kann. Das bedeutet im Umkehrschluss, dass bei deiner Umsetzung insbesondere das Verwalten von Daten nichts zu suchen hat: weder das Laden aus der, noch das Speichern in die Datenbank hat irgend etwas in der Präsentationsschicht zu suchen. Dein UI-Model darf die Datenbank nicht kennen. Außerdem findet die Kommunikation zwischen View und Presenter dadurch statt, dass der Presenter die Datenbindungsquellen des View manipuliert und auf der anderen Seite Ereignisse des View abonniert. Wird natürlich schwierig, das ohne Datenbindung zu machen.

In deinem Beispiel hat der View das Zepter in der Hand. Ich sehe da keinen Gewinn an Organisation gegenüber einem rohen Windoiws-Forms-mit-Code-Behind-Entwurf.

LaTino

Hallo jok3r89,

dein Code entspricht nicht MVP, denn der Presenter steuert ja die UI - und ist kein Teil (Member) von ihr.
Der Sinn ist ja die Austauschbarkeit der UI, d.h. der Presenter entscheidet (d.h. beinhaltet die Logik) welche UI dargestellt wird.

Schau dir mal WinformsMVP sowie den zugehörigen Artikel WinForms MVP - An MVP Framework for WinForms an.

Das ganze ist doch noch nicht so einfach wie ich gedacht habe ich hab mich an ein Beispiel gehalten das ich gefunden hab, aber ich denke ich jetzt kapiert das es von Grund auf falsch war.

>http://www.technical-recipes.com/2015/the-model-view-presenter-pattern-in-c-a-minimalist-implementation/

namespace ThePLCProjekt
{

	public interface IMainForm
	{
		void LoadData(IList<PLCTyp> data);

		PLCTyp addItem { get; }

		event EventHandler AddItemClicked;

		event EventHandler ClearDatabaseClicked;

		event EventHandler UpdateView;
	}

	public partial class MainForm : Form, IMainForm
    {
		#region private
		private MainForm_Presenter _presenter = null;
		#endregion

		#region Events
		public event EventHandler AddItemClicked;
		public event EventHandler ClearDatabaseClicked;
		public event EventHandler UpdateView;
		#endregion

		public MainForm()
        {
            InitializeComponent();
            ListView_Init();
        }

        private void ListView_Init()
        {
            listView1.Columns.Add("id");
            listView1.Columns.Add("name");
            listView1.Columns.Add("ip");
            listView1.Columns.Add("connection");
            listView1.Columns.Add("cycle[ms]");

            // Create the ContextMenuStrip.
            var docMenu = new ContextMenuStrip();
            //Create some menu items.
            ToolStripMenuItem delete = new ToolStripMenuItem();
            delete.Text = "delete";
			//delete.Click += deletePLCToolStripMenuItem_Click;

            ToolStripMenuItem showDBEditor = new ToolStripMenuItem();
			showDBEditor.Text = "db editor";
			showDBEditor.Click += showDBPanel;

            //Add the menu items to the menu.
            docMenu.Items.Add(showDBEditor);
            docMenu.Items.Add(delete);

            listView1.ContextMenuStrip = docMenu;
        }

		public void LoadData(IList<PLCTyp> data)
		{
			listView1.BeginUpdate();
			listView1.Items.Clear();
			foreach (PLCTyp row in data)
			{
				ListViewItem item = new ListViewItem();
				item.SubItems.Add(row.name);
				item.SubItems.Add(Convert.ToString(row.ip));
				item.SubItems.Add(Convert.ToString(row.target));
				item.SubItems.Add(Convert.ToString(row.cycle));
				this.listView1.Items.Add(item);
			}
			listView1.EndUpdate();
		}

		private void btn_Add_Click(object sender, EventArgs e)
		{
			AddItemClicked(this, EventArgs.Empty);
		}

		public PLCTyp addItem
		{
			get
			{
				return
			  new PLCTyp()
			  {
				  name = textBox_name.Text,
				  cycle = Convert.ToInt16(textBox_cylce.Text),
				  ip = textBox_ip.Text,
				  target = comboBox_typ.Text,
			  };
			}
		}

		private void clearDatabaseToolStripMenuItem_Click(object sender, EventArgs e)
		{
			ClearDatabaseClicked(this, EventArgs.Empty);
		}

		private void reloadToolStripMenuItem_Click(object sender, EventArgs e)
		{
			UpdateView(this, EventArgs.Empty);
		}
	}
}

Ich löse jetzt über Events die der Presenter abonniert das Updaten/Manipulieren des Views aus.

@Th69
Ich hab mir auch noch die Beispiele angesehen, leider hab ich damit aber noch ein wenig Probleme ich kann es schon gar nicht erzeugen.

Scheinbar stelle ich mich zu dämlich an, ich finde schon gar nicht wo der Presenter erzeugt wird...

Gruß

Der Presenter (sowie das Model) sollte außerhalb der Form Klasse erstellt werden, d.h. dort wo auch die Form aufgerufen wird, d.h. in der Main-Methode ("Program.cs").

static void Main()
{
	MainForm form = new MainForm();
	Model model = new Model();
	Presenter presenter = new Presenter(form, model);
	presenter.Run(); // ruft z.B. Application.Run(form) auf
}

So ist der Presenter die steuernde Instanz (und könnte z.B. ein anderes Hauptfenster oder auch eine reine Konsolenein-/ausgabe als UI verwenden, z.B. mittels eines Kommandozeilenparameters o.ä.).

PS: In deinem Link ist ja sogar ein Bild, das die View nur per "user events" mit dem Presenter kommuniziert und daher gar nicht den Presenter direkt kennen soll (Verwirrend ist das Bild evtl. weil der Presenter in der Mitte steht (übrigens dasselbe Bild wie in der englischen Wiki) , daher finde ich das Bild in der deutschen Wiki besser).
Und dann schreibt der Autor auch noch, daß er die "Passive View" im Beispiel benutzt, welche "Kopplung zwischen View und Model verbietet", dann aber das Model (noch nicht einmal nur IModel) an die Form weiterreicht...

Reden wir von diesem Beispiel? Dort ist das doch nicht der fall?

 public Form1(Model model)
        {
            m_Model = model;
            InitializeComponent();
            presenter = new Presenter(this, m_Model);
            SubscribeToModelEvents();
        }

Was ich bei dem Git Projekt nicht verstehe ist, wie der Presenter erzeugt wird?
https://github.com/DavidRogersDev/WinformsMVP

Kannst Dir doch das Beispiel ziehen und den Debugger verwenden.
Dafür sind doch Open Source Projekte da: Du siehst alles 😉

Hallo jok3r89,

ja, genau diesen Code meine ich.
Dort wird der UI (Form) das Model (und nicht bloß IModel) übergeben und dann auch noch der Presenter erzeugt -> komplett falsches Design (und dann bezieht sich der Autor auch noch auf "Passive View", wo keine Kopplung zwischen UI und Model bestehen soll)!

Verstehst du denn wenigstens meinen Code [design-technisch]?

Edit:
Bei WinFormsMVP wird der Presenter indirekt (über die View) erzeugt, das steht in dem CodeProject-Artikel unter "A presenter".

Für die wenigsten Programme sehe ich persönlich MVP geeignet, gerade bei reinen WinForms-Projekten würde ich Model View Controller (MVC) benutzen.

Noch ein Nachtrag:
Das man nicht so einfach ein WinForms-Projekt nach z.B. WPF portieren kann, liegt ja auch meistens daran, daß in den Form-Klassen viel zu viel Logik (sowohl "Business Logic" sowie "UI Logic") untergebracht ist. In "reinem" MVP darf in den einzelnen UI-Klassen keine Logik untergebracht sein (z.B. noch nicht einmal der Aufruf eines anderen Fensters, dies wird dann alles über den Presenter gesteuert). Die UI ist nur noch reine Datenanzeige bzw. Datenerfassung.

In "reinem" MVP darf in den einzelnen UI-Klassen keine Logik untergebracht sein (z.B. noch nicht einmal der Aufruf eines anderen Fensters, dies wird dann alles über den Presenter gesteuert). Die UI ist nur noch reine Datenanzeige bzw. Datenerfassung.

Stimmt genau und ist auch der Grund, wieso MVP nicht so beliebt ist. Der Presenter mutiert bei komplexen Oberflächen zu einer Gottklasse und der Mehrgewinn an Organisation geht flöten. Bei MVC ist das besser vermeidbar.

Für Otto-Normal-Oberflächen sollte es aber reichen, ohne dass es undurchschaubar wird.

LaTino

Stimmt genau und ist auch der Grund, wieso MVP nicht so beliebt ist. Der Presenter mutiert bei komplexen Oberflächen zu einer Gottklasse und der Mehrgewinn an Organisation geht flöten. Bei MVC ist das besser vermeidbar.

Für Otto-Normal-Oberflächen sollte es aber reichen, ohne dass es undurchschaubar wird.

LaTino

Wir reden schon vom selben, ich habs verstanden.
Es gibt irgendwie zum Thema MVP einen Haufen an Bsp. aber oft hatten dich sich kaum geähnelt.

Wieso mir auch MVP und nicht MVC in den Sinn gekommen ist, weil ich oft davon gelesen habe das es schwierig ist bei Winforms anzuwenden eben wegen der stricken Trennung.

Und wie man an deinem Beispiel sieht, verstehen einige Leute eben diese Design Patterns nicht richtig (oder machen es sich selbst zu einfach). Die (strikte) Trennung einzuhalten, ist für kleine Projekte auch meist (code-technisch) ein zu großer Aufwand, aber je größer die Projekte werden, umso mehr lohnt es sich.
Heutzutage sollten Unit-Tests gleichwertig zu dem eigentlichen Programmcode erstellt werden, und dann merkt man recht schnell, wo die Trennung noch nicht weitreichend genug ist (wenn man also nicht einfach Mock-Objekte per DI übergeben kann).

Leider machen sich zu wenige Entwickler (sowie Projektleiter) bei Projektstart genug Gedanken für eine langfristige Architektur, die dann auch dauerhaft gepflegt werden muß.

Bei meinem letzten (größeren) Projekt hatten wir auch 2-3 Monate ersteinmal damit verbracht eine vernünftige Build-Chain sowie Grob-Architektur zu erstellen. Auch das hat nicht komplett gereicht, so daß wir während des Projektes noch mal eine größere Designänderung (Einbau einer State-Machine) vornehmen mußten (in gleichen Zuge haben wir dann aber auch softwareseitige Integrationstests geschrieben, so daß wir die Fehlermeldungen unserer Tester sofort nachstellen konnten - seitdem laufen mehr als 50 Integrationstests nach jedem Build in etwas mehr als 1 Minute, während die Tester dafür mind. 1h brauchen - so daß also nur vor großen Releases diese komplett nachgetestet werden).

Ich hab jetzt wieder ein wenig was probiert und mich weiter eingelesen.

Jetzt finde ich dieses Beispiel recht interessant, nur hab ich gerade probleme mit
"Application.Run(form)"
Ich kann dem ja keine Interface übergeben?

Ich habs jetzt mal so gemacht ->

Presenter

		public void Run()
		{
			_view.ApplicationRun();
		}

Form

		public void ApplicationRun()
		{
			Application.Run((Form)this);
		}

Wäre das auch okay?

Gruß

Der Presenter kennt ja direkt alle Forms, kann also direkt diese aufrufen.
Die Form selbst sollte natürlich nicht Applikation.Run aufrufen (wie in einem Standard-Projekt ja auch nicht).

Falls _view bei dir ein Interface ist, dann eben im Presenter

public void Run()
{
    Form form = _view as Form;
    if (form != null)
       Application.Run(form);
    else
      throw new ApplicationException("_view is not a Form!");
}

Okay danke,
und was würde gegen meine Variante sprechen oder wäre das genauso gut?
Gruß

Nein, deine Variante wäre nicht gut, denn eine Form sollte selber keine Abhängigkeit zur aufrufenden Klasse haben (du kannst Application.Run als Presenter bei einem Standard-WinForms Projekt sehen).

Wenn du Lust und Zeit hast, kannst du dir auch mal (besonders die Einleitung) meines Artikels Kommunikation von 2 Forms durchlesen (und evtl. mal das Beispielprojekt ganz unten anschauen).
Dort gehe ich auf den hierarchische Aufbau eines Projektes ein.

Ja danke, ich werde mir das morgen mal zu Gemüte führen.

Ich hänge gerade vor einem neuen Problem und komme seit einer Stunde nicht mehr weiter. Eigentlich bin ich gerade nur dabei die Views mit Events auszustatten. Bisher hat alles funktionier nur aus irgendwelchen gründen bekomme ich es in einem Fall nicht hin.
Ich kann gerade Tipp fehler auch nicht mehr ausschließen nach stunden sieht man den Wald vor lauter bäumen nicht mehr 😄 .

View

public interface IMainForm
{
.......
	event EventHandler DeleteSelectedItemClicked;
}

public partial class MainForm : Form, IMainForm
{
	public event EventHandler DeleteSelectedItemClicked;
.....

private void DeleteSelectedItem_Click(object sender, EventArgs e)
{
     DeleteSelectedItemClicked?.Invoke(this, EventArgs.Empty);
}

....

Im Presenter habe ich das Event zugewiesen

....
	_view.ClearDatabaseClicked += ClearPLCItemSelected;
....

public void ClearPLCItemSelected(object sender, EventArgs e)
{
			var selectedId =Convert.ToInt16(_view.selectedListViewItem[0].Text);
			var plc = this._model.GetPlcByID(selectedId);
			this._model.deletePLC(plc);
			reloadView();
}

DeleteSelectedItemClicked ist aber immer Null. Und ich komme gerade nicht wirklich darauf.
Genauso mache ich das schon gut 10x ohne Probleme und das verwirrt mich gerade.

Gruß

Hast du dich da nicht verschrieben:

 _view.ClearDatabaseClicked += ClearPLCItemSelected;

Du meinst doch sicherlich

 _view.DeleteSelectedItemClicked += ClearPLCItemSelected;

?