Achtung nicht erschrecken. Wenn ich ein Programm auseinander nehme, sieht das immer so aus.
TPerson:
Delphi-Quellcode:
fTag: Integer;
fTyp: String;
Welchen Sinn haben diese Felder? Kommt mir merkwürdig vor.
fMarkiert: Boolean;
OK, ich sehe unten, du verzweigst entsprechend beim Zeichnen. OK.
fMouseOver: Boolean;
dito.
procedure SetTyp(Bezeichnung: String);
string-Parameter sollten const sein.
constructor Create(AOwner: TComponent; Oben: Integer);
Der zusätzliche Parameter Oben kommt mir komisch vor. Damit rechnet man nicht. Mach den weg.
//property _OnDblClickForFamilyTree: [...]
Warum hast du das auskommentiert?
Delphi-Quellcode:
property Tag: Integer read fTag write fTag;
property Typ: String read fTyp write fTyp;
s.o.
Außerdem fehlen TPerson Referenzen auf die Eltern, etc.
TStammbaum:
fGenealogie: TGenealogie; // hier werden die Daten zu den Personen geholt
Zeig mal die Klasse. Zumindest der Bezeichner sieht verbeserungswürdig aus.
fPerson: TObjectList;
Zumindest der Bezeichner ist falsch. Wenn es nur eine Person ist, kann es keien ObjectList sein. Wie aber oben erwähnt, brauchst du keine ObjectList. Zumindest hier nicht.
fProband: Integer;
Nein, nicht Integer. TPerson. Also eine Referenz auf das konkrete Objekt.
fGeneration: Integer;
Was bedeutet dieses Feld?
fMarkiert: Integer;
Auch hier eine Referenz, keine ID.
procedure SetPersonen(Person, Y: Integer);
Parameter sind unverständlich.
procedure PersonenUpdate(Proband_ID: Integer);
Prozedurnamen sind Verben. wenn dann sollte es heißen UpdatePersonen und nicht anders herum. Aber auch dann passt der Name nicht. Denn die Prozedur soll ja den Probanden setzen. Also makeProband() wie in vorherigem Post genannt. Außerdem gilt natürlich auch hier: Vergiss die IDs. Wenn du IDs fürs dateiformat brauchst o.ä. dann lass sie drin. Nimm die aber nur zum Speichern und laden. Ansonsten solltest du mit Referenzen arbeiten.
property Genealogie: TGenealogie read fGenealogie write fGenealogie;
Sicher, dass das Public sein muss?
Delphi-Quellcode:
property Person: TObjectList read fPerson write
property Proband: Integer read fProband write fProband;
property Generation: Integer read fGeneration write fGeneration;
property Markiert: Integer read fMarkiert write fMarkiert;
- properties können auch readonly sein. Manchmal ist das von Vorteil. Hier könnte man sowas überlegen. Insbesonder bei Person... äh... Personen
Delphi-Quellcode:
procedure TPerson.SetTyp(Bezeichnung: String);
begin
Self.fTyp:= Bezeichnung;
Self.Hint:= Bezeichnung;
end;
Das überschreibt den Hint. Das ist so nicht ersichtlich. Wenn, dann solltest du zusätzlich die Möglichkeit bieten, das über ne Property abzustellen. Besser den Hint von außen setzen, wenn nötig. Wie der Hint aussehen soll ist nicht Sache von TPerson.
case Self.fMarkiert of
Bei Boolean-Werten lohnt sich case eigentlich nicht wirklich. if ist da lesbarer
Delphi-Quellcode:
case i of
0: Pers:= TPerson.Create(AOwner, 0); // Proband
1: Pers:= TPerson.Create(AOwner, 170); // Vater
2: Pers:= TPerson.Create(AOwner, -170); // Mutter
3..6: Pers:= TPerson.Create(AOwner, 300); // Großeltern
7..14: Pers:= TPerson.Create(AOwner, 540); // Urgroßeltern
end;
Hm... da gibts jetzt mehrere Sachen zu sagen.
- case ist in wirklich objektorientierten programmen selten. Wenn du es doch brauchst, solltest du zumindest mal überlegen, ob es notwendig ist.
- So langsam beginne ich zu verstehen, wie du das meinst. TPerson ist nicht wirklich eine Person, sondern nur die Grafische Darstellung und die Daten sind irgendwie in TGenealogie? Zeig unbedingt mal TGenealogie her.
- Du hast immer noch nicht erklärt, warum du die Beschränkung auf 4Generationen bzw. 15 Personen hast. Warum willst du niemals mehr anzeigen?
Delphi-Quellcode:
procedure TStammbaum.SetPersonen(Person, Y: Integer);
begin
case Person of
Und was, wenn Person 11 ist? Oder 22? Diese Prozedur ist ziemlich kaputt. Sie sollte viel generischer sein.
Self.Canvas.Pen.Color:= $00A09070;
Für sowas solltest du Konstanten einführen.
Delphi-Quellcode:
procedure TStammbaum.Click;//(Sender: TObject);
var
i: Integer;
begin
//TPerson(Self.Person.Items[1])._OnDblClickForFamilyTree:= PersonDblClicked;
Da gehört das ja auch nicht rein. Das muss beim Erzeugen zugewiesen werden.
Delphi-Quellcode:
procedure TStammbaum.Markieren(Plus: Boolean; Person: Integer);
begin
case Plus of
- plus ist unverständlich
- zudem ist es eine Hybridkopplung. Sollte man vermeiden.
TINDI(Person.Items[0])
Hier castest du TPerson-Objekte auf TINDI. Das ist merkwürdig und problematisch. Sei überhaupt vorsichtig mit casts. Vermeide sie wos nur geht.
Exit;
exit ist nicht gut. Vermeide es. exit kann man für so genannte Wächter verwenden. Also ganz am Anfang einer Methode. Aonsten solletst du exit vermeiden,w eil es ein Programm tendenziell unleserlich macht.
=========================================
Langer Rede kurzer Sinn: Ich denke wir sollten nochmal Schritt für Schritt von vorne anfangen. Dann kriegen wir auch etwas Sauberes hin. Das wird ein bisschen Arbeit, aber du solltest dadurch einiges lernen (und ich auch). Lies trotzdem mal meine Kommentare oben und versuche, sie zu verstehen.
Zeig erstmal TGenealogie und TINDI. Dann bau ich dir mal ein Grundgerüst.
mfg
Christian