Opensource Lichtsteuerung PC_DIMMER2008

[Robert Marquardt]

Kommen wir mal zum schwachen Verstaendnis von booleschen Ausdrucecken.

Delphi-Quellcode:

procedure TMainForm.CheckBox1MouseUp(Sender: TObject; Button: TMouseButton;
  Shift: TShiftState; X, Y: Integer);
begin
  if checkbox1.checked=true then data.manfade[(data.page*8)+1]:=true else data.manfade[(data.page*8)+1]:=false;
  if checkbox2.checked=true then data.manfade[(data.page*8)+2]:=true else data.manfade[(data.page*8)+2]:=false;
  if checkbox3.checked=true then data.manfade[(data.page*8)+3]:=true else data.manfade[(data.page*8)+3]:=false;
  if checkbox4.checked=true then data.manfade[(data.page*8)+4]:=true else data.manfade[(data.page*8)+4]:=false;
  if checkbox5.checked=true then data.manfade[(data.page*8)+5]:=true else data.manfade[(data.page*8)+5]:=false;
  if checkbox6.checked=true then data.manfade[(data.page*8)+6]:=true else data.manfade[(data.page*8)+6]:=false;
  if checkbox7.checked=true then data.manfade[(data.page*8)+7]:=true else data.manfade[(data.page*8)+7]:=false;
  if checkbox8.checked=true then data.manfade[(data.page*8)+8]:=true else data.manfade[(data.page*8)+8]:=false;
end;

Abgesehen das man nicht explizit auf "= true" abfragen muss, geht es natuerlich so viel einfacher:

Delphi-Quellcode:

procedure TMainForm.CheckBox1MouseUp(Sender: TObject; Button: TMouseButton;
  Shift: TShiftState; X, Y: Integer);
begin
  data.manfade[data.page*8+1] := CheckBox1.Checked;
  data.manfade[data.page*8+2] := CheckBox2.Checked;
  data.manfade[data.page*8+3] := CheckBox3.Checked;
  data.manfade[data.page*8+4] := CheckBox4.Checked;
  data.manfade[data.page*8+5] := CheckBox5.Checked;
  data.manfade[data.page*8+6] := CheckBox6.Checked;
  data.manfade[data.page*8+7] := CheckBox7.Checked;
  data.manfade[data.page*8+8] := CheckBox8.Checked;
end;

Die Checkboxen in einem Array wuerden das jetz nochmals deutlich vereinfachen.
Im Uebrigen ist dies ein klassischer Fall von Copy & Paste Code. An einer anderen Stelle habe ich genau die gleichen Zeilen gesehen.
Dies ist auch offensichtlich eine Basisaktion im Code. Es sollte eine eigene Methode geschrieben werden, um die Aktion zu kapseln.

Der naechste Fehler liegt in Zeilen wie dieser:
  device1.Caption:=inttostr(strtoint(device1.Caption)+4); Hier wird ein Wert in der Caption der Control aufbewahrt. Das ist schlechter Stil, da keine Trennung von Controls und Algorithmus vorliegt.
Besser den Wert getrennt in einer Variablen aufbewahren. Die Funktionsweise des Programms sollte die Eingaben des Benutzers in eine
Zustandsaenderung der Variablen = Eingabewerte umsetzen und dann die Algorithmen aufrufen. Diese aendern den Zustand, der dann auf die
Benutzeroberflaeche zurueckuebertragen wird.

Man koennte zumindest die Tag-Property ausnutzen, aber fuer diese gibt es eine bessere Verwendung. Indem man dort den Index des
Control-Arrays ablegt, kann man mit Ausdruecken der Art "Checkboxes[TCheckbox(Sender).Tag]" allen diesen Checkboxen einen einzigen Handler geben.

Delphi-Quellcode:

procedure TMainForm.CheckBox1MouseUp(Sender: TObject; Button: TMouseButton;
  Shift: TShiftState; X, Y: Integer);
begin
  data.manfade[data.page*8 + Checkboxes[TCheckBox(Sender).Tag] := TCheckBox(Sender).Checked;
end;

Damit wird auch klar das die Uebertragung aller acht Werte Overkill ist.