Bewertung der 2. Adressverwaltung-Abgabe (Splitter, ListControl)

Häufige Kritikpunkte:

• Scheinbar macht das Befüllen des CListControl Schwierigkeiten, da der CString des CAdresse-Namens nicht in 'LPSTR' konvertiert werden kann. Die Lösung hierzu bringt folgendes Makro (ein Feature der MFC 7, dank an die FH für das Upgrade):
  

  	LPTSTR szr = CA2T(adr->getName() );
  	lvItem.pszText = szr; 

  Erklärung: "LP" steht für Pointer, "STR" für String (ach nee...). Und das dazwischen kann den Wert "A" (ANSI character string), "W" (Unicode character string) und "T" (Generic character string (equivalent to W when _UNICODE is defined, equivalent to A otherwise) ) annehmen. CString erlaubt nur Konvertierungen nach LPTCSTR ("C" = constant), wir brauchen hier aber einen editierbaren Buffer. Deshalb gibt es diverse Kopiermakros, die einen PXSTR String (das X steht für A, W oder T und ist der ominöse Operator "PXSTR" in der Klasse CString) in einen anderen String in einem anderen Charset kopieren können. Da wir im Default Unicode nicht aktiviert haben, können wir den Ansi-String "Name" in einem generischen String kopieren (mittels "CA2T"), diesen wiederum können wir ins LVITEM packen.
  Problem bei obigem Beispiel: Klappt nur bis zu einer Länge von 128 Zeichen. Abhilfe schafft das Makro "CA2TEX", dem wir als Template-Parameter eine Maximallänge (leider konstanter Wert) übergeben können.

  	LPTSTR szr = CA2TEX<1024>(adr->getName() ); 

  Nach cleverer geht es, wenn wir die Klasse "CA2T" verwenden, die einen String kapselt.

  	CA2T pszA( adr->getName() );
  	lvItem.pszText = pszA; 

  Der Tooltip zu CA2T verrät folgendes "#define CA2T CA2W", d.h. intern wird auf Unicode umgestellt.
  Die Doku findet man Inhalt-Baum an dieser Stelle:
  MSDN Library 2003: "Visual Studio .NET" / "Visual C++" / "Referenz" / "Visual C++ Libraries" / "ATL Refrence" / "ATL Macros" / "String Conversion Macros"
  MSDN Library Januar 2005: "MSDN Library-January 2005" / "Development Tools and Languages" / "Visual Studio .NET" / "Product Documentation" / "Visual C++" / "Reference" / "Visual C++ Libaries" / "ATL Reference" / "ATL Macros" / "ATL Macros Alphabetical Reference" / "ATL and MFC String Conversion Macros"
  
  Noch einfacher geht es mittels Typecasts (dass ein CString nur einen konstanten String zurückgeben kann stört ja nicht, wenn man diesen einfach in einen modifizierbaren String casten kann):

  	lvItem.pszText = (LPSTR) (LPCTSTR) adr->getName(); 

• Dialog für Adresseneingabe: OnOK muss nicht überladen werden, wenn folgendes in DoDataExchange eingefügt wird:

  	//Nur beim Save+Validate die Prüfung auf Nachname/Vorname:
  	if (pDX->m_bSaveAndValidate == TRUE)
  	{
  		if(m_strName == "" && m_strVorname == "")
  		{
  			AfxMessageBox("Name oder Vorname dürfen nicht leer sein", MB_OK);
  			pDX->Fail ();
  		}
  	} 

146 006

• Wieso zwei Beispieleinträge, die beim Beenden auch noch Memory Leaks hinterlassen haben ?
• DeleteContents des Documents ist nicht überladen (Adressen werden beim Beenden nicht gelöscht). In der ersten Abgabe hattest du das richtig.
• Löschen ohne gewählte Adresse: es passiert nix. Hier bitte eine entsprechende Fehlermeldung.
• Bearbeiten ohne gewählte Adresse entspricht "Neu anlegen". Hier wäre eine Fehlermeldung "nichts zum Bearbeiten gewählt" wahrscheinlich intuitiver.
• Mein Fehler: das MFC-Beispiel Modified-Flag hatte einen Fehler, auch "DoSave" muss überladen werden (siehe dort).
In AdressenZweiDoc.h sind die überladenen Methoden aus CDocument sehr seltsam deklariert:

	BOOL CAdressenZweiDoc::DoFileSave();
	BOOL CAdressenZweiDoc::SaveModified(); 

"CAdressenZweiDoc::" hat hier nichts zu suchen.
• CAdressenZweiDoc::Serialize: Beim Laden kein Aufruf von DeleteContents/ UpdateAllViews. Macht MFC automatisch.
• Es fehlen die Kommentare zu den Methoden bzw. sind unvollständig (wie JavaDoc: vor jeder Methode erklären was sie macht und was die Parameter und Rückgabewerte bedeuten). Beispiel: CAdressenZweiDoc::getNextElement beschreibt nur Rückgabewert aber nicht was die Methode macht. CAdressenZweiDoc::CreateNewPerson hat keinen Methodenkommentar. Teilweise stehen diese Kommentare in der ersten Zeile der Methode, dies dann nach außen ziehen. Die LeftView braucht zu den ganzen OnXXX-Methoden ebenfalls Kommentare.
• OnBnClickedButtonNeu: Wieso eine Schleife ? Das gesamte WHILE und bDialogFertig kannst du weglassen.
• CAdressenDialog::OnOK muss nicht überladen werden, siehe "Häufige Kritikpunkte"
• Kritikpunkt aus letzter Abgabe: CAdressenDoc.CreateNewPerson: wieso beim Ersetzen eine vorhandene Person löschen statt sie zu recyclen ?
• Styleguide: Methodenparameter wie "NewPerson" sind nicht das Gelbe vom Ei. Wenn schon dann Kleinschreibung und Datentyp in Name, also z.B. "personNew" oder "p_personNew".

546 873

• AddressNotFoundException nicht inline im Document, sondern als eigene Klasse/Datei. Dann findet man sie leichter.
• CAdressenSplitterDoc::GetAddress: Ist zwar berechtigt dass hier eine Exception geworfen wird wenn nicht gefunden. Allerdings kann z.B. CAdressenSplitterRightView::OnDraw garnicht ohne Exception herausfinden ob eine Adresse gewählt ist. Deshalb sollte "getSelectedAddress" im Fehlerfall NULL zurückliefern. Außerdem wäre es schön wenn es eine Methode "ContainsAdresse (sName, sVorname)" gäbe, die einen BOOL-Wert zurückliefert. Dann könnte von außerhalb ohne Exception geprüft werden ob eine Adresse existiert. Exceptions nur für ECHTE Fehlerzustände aber nicht für etwas mit dem man rechnen muss.
  Beispiel: Öffnen einer Datei: Vor dem Öffnen prüfen ob sie existiert (dadurch Exception vermeiden). Die Öffnen-Methode sollte natürlich trotzdem auf Vorhandensein prüfen und evtl. eine Exception werfen wenn nix da ist (weil z.B. durch unsaubere Programmierung im Aufrufer dieser Fall nicht abgefangen wurde). Falls allerdings während des Lesens ein Fehler auftritt (Datei wurde in diesem Moment gelöscht/geändert), dann ist eine Exception völlig gerechtfertig, weil das ein Aufrufer nicht selbst sicherstellen kann und nicht damit rechnen kann.
• Styleguide: CAdressenSplitterDoc::getSelectedAddress und andere (Methodennamen groß schreiben).
• Mein Fehler: das MFC-Beispiel Modified-Flag hatte einen Fehler, auch "DoSave" muss überladen werden (siehe dort).
• Bearbeiten und Ändern des Namens: erzeugt eine neue Adresse statt die alte zu ändern. Das wäre in dem Fall kein "Bearbeiten" mehr.
• Neues Document hat direkt den Stern im Titel gesetzt. Schuld ist scheinbar "mapRemoveAll", das aus "DeleteContents" heraus aufgerufen wird, und dies wiederum aus "CDocument::OnNewDocument"
• Mehrfach auf "Datei -> Neu" gehen: rechte View schrumpft schrittweise ;-)
• CAdressEingabeView::OnOK muss nicht überladen werden, siehe "Häufige Kritikpunkte".

546 881

• Die Eingabe der Daten sollte eigentlich in einem separaten Dialog geschehen, nicht in der FormView selbst ! Beim Klick auf Neu/Bearbeiten soll sich dieser Dialog öffnen.
• Die rechte View wird überhaupt nicht genutzt.
• Löschen einer Adresse: in den Textfeldern stehen immer noch die Daten der gelöschten Adresse. Die Felder bitte leeren.
• "SetModified" des Documents wird nicht verwendet (automatische Speichern-Abfrage beim Beenden).
• Bei Eingabe von z.B. "a0" bei der Postleitzahl gibt es die Meldung "Ungültige Eingabe", aber sonstige Änderungen werden übernommen. In OnBnClickedButtonUebernehmen muss die erste Zeile lauten:

  	if (UpdateData(TRUE) ) == FALSE) 
  		return;  

• Namenskonventionen: "a2" als Beginn von Klassennamen ist gegen den Styleguide, Klassenname müssen mit Großbuchstaben beginnen, also mindestens "A2...", besser "Adressen..." (weil "A2" bei der nächsten Abgabe nicht mehr stimmt, und du wirst es bestimmt nicht freiwillig in "A3" ändern wollen).
• Tipp: in "OnBnClickedButtonUebernehmen" und "OnUpdate" kannst du dir die Mords-Arbeit (und die FUCK MSsens) sparen, wenn du den Nachnamen setzt wie unter "Häufige Kritikpunkte" beschrieben. Der Aufwand über TCHAR[] ist nicht nötig, und sogar gefährlich, denn falls der User irgendwann mal mehr als 1024 Zeichen eingibt, dann hast du einen Buffer Overflow.
• Document.Serialize: Beim Laden ist der Aufruf von DeleteContents/UpdateAllView nicht nötig !
• OnUpdate: das Wiederherstellen der alten Auswahl funktioniert nicht, statt dem "<" muss in der Abfrage ein ">" stehen:

  	if ((-1 != iSelection) && (m_ListCtrl.GetItemCount() > iSelection)) 

• Positiv: Gute Kommentierung !
  Komfort: vorherige Auswahl in ListBox wird nach Refill wiederhergestellt.
  Alle Kritikpunkte der letzten Abgabe erledigt.

845 927

• Styleguide: Konstanten (im OnDraw) komplett groß schreiben und besser in der Header-Datei definieren.

944 689

• Hier gelten die gleichen Kritikpunkte wie bei 546 881. Schön dass ihr schon die Teamarbeit übt ;-).
  Unterschiede:
  Klassen folgen der Namenskonvention.
  Document.Serialize ist korrekt (kein DeleteContents und UpdateAllViews)