Vereinfachung gesucht

[Die Muhkuh]

Hi,

ich hab hier in einem Code mehrere Stellen (ca. 30 Stück), die vom Prinzip so aussehen:

Delphi-Quellcode:

if RadioButton1.Checked then
begin
  if CheckBox1.Checked then
    SQL.Add('abc')
  else
  if CheckBox2.Checked then
    SQL.Add('def') 
  else
  begin
    SQL.Add('abc'); 
    SQL.Add('def');
  end; 
end;

Hat jemand eine sinnvolle Idee, sich da die Redundanz zu sparen?

[himitsu]

Können beide CheckBoxen gleichzeitig markiert sein?

wenn nicht:

Delphi-Quellcode:

if RadioButton1.Checked then
begin
  if not CheckBox1.Checked then SQL.Add('def');
  if not CheckBox2.Checked then SQL.Add('abc');
end;

PS:

Delphi-Quellcode:

procedure DoThis(Group: TRadioButton; Check1, Check2: TCheckBox; SQL: TIrgendwas; Add1, Add2: String);
begin
  if Group.Checked then
  begin
    if Check1.Checked then
      SQL.Add(Add1)
    else
    if Check2.Checked then
      SQL.Add(Add2)
    else
    begin
      SQL.Add(Add1);
      SQL.Add(Add2);
    end;
  end; 
end;

[blackfin]

mhm, oder so vielleicht?

Delphi-Quellcode:

var 
 mode: Integer;
...
..
begin
 mode := 0 ;
 if CheckBox1.Checked then Inc(mode) 
 else if CheckBox2.Checked then mode := mode +2 ;

 if ( (mode = 0) or (mode = 1) ) then SQL.Add('abc') ;
 if ( (mode = 0) or (mode = 2) ) then SQL.Add('def') ;
end;

alternativ auch mit zwei case:

Delphi-Quellcode:

var 
 mode: Integer;
...
..
begin
 mode := 0 ;
 if CheckBox1.Checked then Inc(mode) 
 else if CheckBox2.Checked then mode := mode +2 ;

 case mode of
   0,1: SQL.Add('abc') ; 
 end;

 case mode of
   0,2: SQL.Add('def') ; 
 end;
 
end;

[Die Muhkuh]

Hi,

@Fin: Interessanter Ansatz, schau ich mir gleich mal näher an.

@Frank: Nein, entweder ist keine aktiv oder nur eine davon. Deine erste Methode probier ich auch gleich mal aus.

Die Funktion fällt leider weg, da manchmal 2-3 SQL-Anweisungen kommen oder die Prüfung auf die Checkboxen entfällt.

[himitsu]

@blackfin: wenn, dann doch eher so: (diese CASE würden es nicht grad vereinfachen)

Delphi-Quellcode:

if RadioButton1.Checked then begin
  if CheckBox1.Checked then mode := 1
  else if CheckBox2.Checked then mode := 2 else mode := 0;
  if mode in [0, 1] then SQL.Add('abc');
  if mode in [0, 2] then SQL.Add('def');
end;

[add]
CheckBoxen optional (einfach nil übergeben) und mehrere Strings:
(kann man auch noch mit überladenen Proceduren und/oder Default-Parametern aufmotzen und mit der "mode"-Variante kombinieren)

Delphi-Quellcode:

procedure DoThis(Group: TRadioButton; Check1, Check2: TCheckBox; SQL: TIrgendwas; Add1, Add2: array of String);
var
  i: Integer;
begin
  if Group.Checked then
    if Assigned(Check1) and Check1.Checked then begin
      for i := 0 to High(Add1) do SQL.Add(Add1[i]);
    end else if Assigned(Check2) and Check2.Checked then begin
      for i := 0 to High(Add2) do SQL.Add(Add2[i]);
    end else begin
      for i := 0 to High(Add1) do SQL.Add(Add1[i]);
      for i := 0 to High(Add2) do SQL.Add(Add2[i]);
    end;
end;

[edit]
Schade,

Delphi-Quellcode:

for Add1 in S do SQL.Add(S);

geht nicht.

[blackfin]

Stimmt, das ist noch besser :)
Naja, es ging ja in erster Linie um die Redundanz-Vermeidung, das tut mein Code auch, aber deiner ist noch eleganter :D

[Die Muhkuh]

So, da Franks Lösung funktioniert, brauch ich nicht mehr weiter testen, weil es ja die einfache Lösung ist (hätte fast von mir sein können :stupid: )

[himitsu]

PS: Noch was zu den CheckBoxen (falls sowas nicht schon eingebaut ist).

So kann man nur eine CheckBox anklicken und vorallem sieht man auch gleich, daß die andere dann nicht mehr geht.

Delphi-Quellcode:

// ins OnClick beider CheckBoxen
procedure TForm9.CheckBox1Click(Sender: TObject);
begin
  CheckBox2.Enabled := not CheckBox1.Checked;
  CheckBox1.Enabled := not CheckBox2.Checked;
end;

[Die Muhkuh]

Ich verwende die ComboBox von DevExpress, die haben das schon on Board, dass man nur eine klicken kann :-)

[DeddyH]

Zitat:

Zitat von himitsu (Beitrag 1046953)

Delphi-Quellcode:

// ins OnClick beider CheckBoxen
procedure TForm9.CheckBox1Click(Sender: TObject);
begin
  CheckBox2.Enabled := not CheckBox1.Checked;
  CheckBox1.Enabled := not CheckBox2.Checked;
end;

Und dann im OI beide auf Checked setzen :mrgreen:

[edit] Nee, das bringt ja nix. Spaß macht es nur, wenn man nur eine der beiden im OI checked und die andere dann im OnShow ^^ [/edit]

[generic]

Checkboxen sollten alleine funktionieren!

Wenn nur eine an sein soll, dann hat die Windowsoberfläche Radioboxen dafür.

Alles anderen ist am GUI-Standard vorbei und kann den User irritieren!

[Die Muhkuh]

Das mit den Checkboxen war eigentlich nur ein Beispiel, um das Prinzip zu verdeutlichen. Eigentlich sind es SpeedButtons ;-)

Mit der Radiobox wählt man ein Gerät aus, das man suchen möchte, und die SpeedButtons bestimmen ob man entweder elektische Abtauung ("CheckBox1 checked" / "SB1 down"), keine Abtauung ("checkbox2 checked" / "SB2 down") oder ob es egal ist (SB1 und SB2 nicht down).

Passt schon so :-)

[gravedigger]

Hallo Muhkuh,

bei SpeedButtons würde ich es mit GroupIndex und AllowAllup versuchen.

Gruß
Gravedigger

[Die Muhkuh]

Das ist doch alles schon passiert... Es ging nur um diese Vereinfachung. Die UI an sich reagiert so, wie sie reagieren soll.

[gravedigger]

Und wenn du die Auswertung über Actions machst.
Die kann man ja, wenn immer das gleiche passiert und man über TAction(Sender) geht, mehrfach verwenden.

[Die Muhkuh]

Das würde bedeuten, die halbe Anwendung umzuschreiben. Franks Lösung funktioniert wunderbar, ansonsten gibt es hier jetzt nix mehr zu diskutieren.