Klassendesign. Frage zu Property Settern was sie dürfen und was nicht

[Bünni]

State enthält einem Enum. Icon enthält den eigentlichen Integer Index für die ImageList den ich einmalig im Setter aus dem übergebenem Enum ermittle.

Ich habe diese beiden Properties der Einfachheit halber gewählt.
Da wo ich den IconIndex brauche (lesend), kann ich direkt auch auf IconIndex zugreifen und erhalte den Integer Wert zurück.
Wo ich nur den Status prüfen muss (lesend), kann ich State auslesen und erhalte ein Enum zum Vergleich zurück.

TStatus = (tsCalculating, tsError Wenn ich State auf tsError setze, dann bekommt IconIndex im SetKSTState-Setter den Wert Ord(Value);
Das hat wie oben schon erwähnt den Vorteil, dass ich direkt an das IconIndex als Integer komme und nicht erst umrechnen müsste, welches Element es in TStatus ist.

Die Redundanz habe ich also entweder im Klassenobjekt einmalig, oder mehrfach dort, wo ich sonst das IconIndex brauche und erst vom State umrechnen müsste.
IconIndex wird von Außen nur gelesen, nicht gesetzt.


Kurz zusammengefasst könnte man meine Frage auch so schreiben:
"darf ein Property 1 in seinem Setter ein anderes Property 2 ändern?"

Die frage hast du mir beantwortet. Damit ist für mich jetzt klar, dass das kein Problem ist.

[Aviator]

Also ich sehe das so, dass du den IconIndex gar nicht setzen brauchst. Es gibt auch Readonly Properties die nur einen Getter haben. Dann würde es reichen, einfach im Getter der IconIndex Property anhand des Status den richtigen Index zurückzugeben. Vorteil hier ist, dass du im SetStatus nur die Variable für den eigentlichen Status ändern musst, die Property sich nur um sich selbst kümmern muss (keine größeren Abhängigkeiten schaffen) und nicht noch durch irgendeine Setter Logik plötzlich eine andere Property geändert wird. Und du hast eine Variable weniger im Objekt.

Um das aber ganz genau zu machen, dürfte dein Objekt gar keine IconIndex Property besitzen. Das Objekt geht es nämlich nichts an, wie der Status später angezeigt wird. Ob das ein TEdit ist in dem ein anderer Text erscheint oder ein Image das die Farbe verändert spielt dann keine Rolle mehr. Alle orientieren sich nur noch am Status Flag des Objekts. Und gerade beim VirtualTreeView ist das ja relativ einfach. Einfach InvalidateNode() aufrufen und im OnGetImageIndex() bzw. On[Before|After]CellPaint() Event anhand des Status den entsprechenden Index der aktuellen ImageList ermitteln und zeichnen lassen.

Probleme würdest du schon bekommen, wenn du deine Klasse an zwei verschiedenen Stellen im Programm verwenden wolltest die zwei unterschiedliche ImageLists benutzen. Dann wäre nämlich deine komplette Speicherung des ImageIndex im Objekt nutzlos.

[Bünni]

Zitat:

lso ich sehe das so, dass du den IconIndex gar nicht setzen brauchst. Es gibt auch Readonly Properties die nur einen Getter haben. Dann würde es reichen, einfach im Getter der IconIndex Property anhand des Status den richtigen Index zurückzugeben.

Werde ich mir ansehen.

Zitat:

Um das aber ganz genau zu machen, dürfte dein Objekt gar keine IconIndex Property besitzen.

Zitat:

Und gerade beim VirtualTreeView ist das ja relativ einfach. Einfach InvalidateNode() aufrufen und im OnGetImageIndex() bzw. On[Before|After]CellPaint() Event anhand des Status den entsprechenden Index der aktuellen ImageList ermitteln und zeichnen lassen.

Wenn ich hier anhand des Status den Index ermitteln möchte, muss ich aber statt KSTData.IconIndex > ord(KSTData.Status) verwenden. Ist das so richtig?

[Aviator]

Naja wenn du dir sicher bist, dass in deiner ImageIndex die Icons immer an den Positionen 0..n liegen kannst du das so machen. Das bedeutet im Umkehrschluss aber auch, dass du bei einem neuen Status immer etwaige andere Icons verschieben und dann auch im Code die Änderungen nachziehen musst. Es sei denn, die ImageList wird nur für diese Status Images benutzt. Aber auch das halte ich für eine etwas unsaubere Implementierung.

Lagere die Ermittlung des ImageIndex lieber in eine eigene Funktion aus. Dann hast du das zentral gespeichert und bist total flexibel. Zudem verkürzt sich dein OnGetImageIndex() auf eine einzelne Zeile zur Ermittlung des ImageIndex (abgesehen vom Abrufen des NodeData Objekts). Zudem erhälst du in dem Beispiel auch direkt eine Exception statt eines falschen ImageIndex wenn du einen neuen Status einführst und vergisst, diesen in der GetImageIndexFromStatus() Methode nachzuziehen.

Delphi-Quellcode:

function GetImageIndexFromStatus(const AStatus: TStatus);
begin
  Result := -1;
  
  case AStatus of
    tsCalculating: Result := 0;
    tsError: Result := 1;
    tsSomeNewStatus: Result := 37;
    else
      raise ENotImplemented.CreateFmt('StatusCode %d not implemented.', [Ord(AStatus)])
  end;
end;

[Bünni]

Zitat:

Naja wenn du dir sicher bist, dass in deiner ImageIndex die Icons immer an den Positionen 0..n liegen kannst du das so machen. Das bedeutet im Umkehrschluss aber auch, dass du bei einem neuen Status immer etwaige andere Icons verschieben und dann auch im Code die Änderungen nachziehen musst. Es sei denn, die ImageList wird nur für diese Status Images benutzt. Aber auch das halte ich für eine etwas unsaubere Implementierung.

Deswegen habe ich den IconIndex im Setter oben ja auf Ord(Value [State]) gesetzt. Somit war er immer gleich. Das Problem mit sich verschiebenen Indexes hatte ich nie.

Zitat:

Zudem verkürzt sich dein OnGetImageIndex()

Ich male meine Icons selber. OnGetImageIndex benutze ich nicht.

Helferfunktionen werde ich gleich einbauen so wie du vorgeschlagen hast. Das klingt gut und so werde ich auch ein paar Properties los.


@Aviator
deine Änderungen sind eingebaut. Alle Abhängigkeiten sind verschwunden, Helferfunktionen eingebaut und IconIndex ist auch weg.
Änderungen am Klassenobjekt
- 3 Feldvariablen weniger
- 6 Properties weniger
- 36 Zeilen Code ohne Leerzeilen weniger
- 3 Helferfunktionen sind dazugekommen die all die vorherige Arbeit in jeweils einer Zeile erledigen.

Die Unit mit meinem Klassenobjekt hat zwar noch immer 953 Zeilen mit Leerzeilen und ein paar Kommentaren, aber da steckt auch viel drin. Zusammen 92 Properties.
Keine Redundanzen, keine doppelten Properties. Alles wichtige Dinge für den Programmablauf.