bessere lösung für prozeduren

[xan553]

servus,

gibt es für solche codepassagen vielleicht bessere lösungen die nicht so massig sind?

Delphi-Quellcode:

procedure TTblLieferanten.setSearchLabels(m: Byte);
begin
  case m of
    1: begin
         combobox2.Clear;
         Label10.visible := True;Label10.Caption := '1 = Name';
         Label11.visible := True;Label11.Caption := '2 = Kunden Nr.';
         Label13.visible := True;Label13.Caption := '';
         Label14.visible := True;Label14.Caption := '';
         combobox2.Items.Add('1');
         combobox2.Items.Add('2');
         combobox2.ItemIndex := 0;
       end;
    2: begin
         combobox2.Clear;
         Label10.visible := True;Label10.Caption := '1 = Name';
         Label11.visible := True;Label11.Caption := '3 = Artikel Nr.';
         Label13.visible := True;Label13.Caption := '6 = Bezeichnung';
         Label14.visible := True;Label14.Caption := '7 = Lieferanten';
         combobox2.Items.Add('1');
         combobox2.Items.Add('3');
         combobox2.Items.Add('6');
         combobox2.Items.Add('7');
         combobox2.ItemIndex := 0;
       end;
    3: begin
         combobox2.Clear;
         Label10.visible := True;Label10.Caption := '2 = Kunden Nr.';
         Label11.visible := True; Label11.Caption := '5 = Rechnungs Nr.';
         Label13.visible := True; Label13.Caption := '';
         Label14.Caption := '';
         combobox2.Items.Add('1');
         combobox2.Items.Add('2');
         combobox2.Items.Add('5');
         combobox2.ItemIndex := 0;
       end;
    4: begin
         combobox2.Clear;
         Label10.visible := True;Label10.Caption := '1 = Name';
         Label11.visible := True;Label11.Caption := '2 = Kunden Nr.';
         Label13.visible := True;Label13.Caption := '4 = Auftrags Nr.';
         Label14.Caption := '';
         combobox2.Items.Add('1');
         combobox2.Items.Add('2');
         combobox2.Items.Add('4');
         combobox2.ItemIndex := 0;
       end;
    5: begin
         combobox2.Clear;
         Label10.visible := True;Label10.Caption := '1 = Name';
         Label11.Caption := '';
         Label13.Caption := '';
         Label14.Caption := '';
         combobox2.Items.Add('1');
         combobox2.ItemIndex := 0;
       end;
end;

[stahli]

In dem Umfang würde ich es auch so machen.

Bei komplexeren Varianten könnte man die Daten (Visibal, Caption etc.) z.B. in einer Ini bereitstellen, um den Quelltext zu minimieren.
Ansonsten käme noch die Verwendung von PageControls (mit ausgeblendeten Registern) oder eingebette Formulare bzw. Frames in Frage.
Das würde jedoch mehr Aufwand machen, als Deine Lösung.

[TiGü]

0. "m" ist kein guter Name für einen Prozedur-/Funktionsparameter, bitte sprechende Bezeichner verwenden.

1. Nicht einfach Zahlen als Parameter für die case-Anweisung verwenden, sondern einen eigenen Aufzählungstyp verwenden.

2. Combobox2.clear kann vor dem case hingeschrieben werden, da es in allen Fällen vorkommt.

2.1. Ebenso combobox2.ItemIndex := 0, dann aber nach der case-Anweisung.

3. Strings als const oder resource strings am Anfang der Unit deklarieren. Lässt sich so auch besser kombinieren.

4. Combobox2.Items.Delimiter und Combobox2.Items.DelimitedText würde ich mir an deiner Stelle mal ganz genau anschauen. :)

5. Müssen Labels, die einen leeren String zugewiesen bekommen, wirklich visible sein?

5.1 Müssen unsichtbare Labels einen leeren String zugewiesen bekommen?

6. Das setzen der visible-Eigenschaft für die Labels 10 bis 13 kann vor der case-Anweisung geschehen oder in einer Subroutine gemacht werden.
Nur im letzten Fall kann man die dann explizit unsichtbar setzen (obwohl ohnehin leer).

Bei richtiger Umsetzung werden am Ende dann so rund nur drei bis fünf Anweisungen pro Fall übrigbleiben.

[dataspider]

Ich würde hier schon Code in eine Procedure auslagern...

Delphi-Quellcode:

procedure TTblLieferanten.setSearchLabels(m: Byte);
  procedure SetItem(ALabel: TLabel; const ANr: string = ''; const AFunction:
      string = '');
  begin
    ALabel.Visible := True;
    if AFunction <> '' then
      ALabel.Caption := ANr + ' = ' + AFunction;
    if ANr <> '' then
      Combobox2.Items.Add(ANr);
  end;

begin
  combobox2.Clear;
  case m of
    1:
      begin
        SetItem(Label10, '1', 'Name');
        SetItem(Label11, '2', 'Kunden Nr.');
        SetItem(Label13);
        SetItem(Label14);
      end;
    2: ...
  end;
  combobox2.ItemIndex := 0;
end;

Wichtig sind allerdings auch die Hinweise von TiGü...

Frank

[Bummi]

Hübscher, besser wartbar fände ich etwas in der Art:

Delphi-Quellcode:

implementation

{$R *.dfm}

type
TInfoRec=Record
  Nr:Integer;
  Caption:String;
End;

const
InfoRecArray:Array[1..5,1..4] of TInfoRec=(
                                         (
                                          (Nr : 1;Caption :'Name')
                                          , (Nr : 2;Caption :'Kunden Nr')
                                          , (Nr : -1;Caption :'')
                                          , (Nr : -1;Caption :'')
                                          ),
                                          (
                                          (Nr : 1;Caption :'Name')
                                          , (Nr : 3;Caption :'Artikel Nr')
                                          , (Nr : 6;Caption :'Bezeichnung')
                                          , (Nr : 7;Caption :'Lieferant')
                                          ),
                                          (
                                          (Nr : 2;Caption :'Kunden Nr')
                                          , (Nr : 5;Caption :'Rechnungs Nr')
                                          , (Nr : -1;Caption :'')
                                          , (Nr : -1;Caption :'')
                                          ),
                                          (
                                          (Nr : 1;Caption :'Name')
                                          , (Nr : 2;Caption :'Kunden Nr')
                                          , (Nr : 4;Caption :'Auftrags Nr')
                                          , (Nr : -1;Caption :'')
                                          ),
                                          (
                                          (Nr : 1;Caption :'Name')
                                          , (Nr : -1;Caption :'')
                                          , (Nr : -1;Caption :'')
                                          , (Nr : -1;Caption :'')
                                          ));

procedure TForm1.DisplayArray(Nr:Integer);
var
  i:Integer;
  l:TLabel;
begin
  Combobox1.Items.Clear;
  for I := 1 to 4 do
      begin
         l := TLabel(FindComponent('Label' + IntToStr(i)));
         if Assigned(l) then
            begin
              l.Visible :=  InfoRecArray[Nr,i].Nr >0;
              l.Caption :=  IntToStr(InfoRecArray[Nr,i].Nr) + ' = ' + InfoRecArray[Nr,i].Caption;
              if l.Visible then Combobox1.Items.Add(IntToStr(InfoRecArray[Nr,i].Nr));
            end;
      end;
   Combobox1.ItemIndex := 0;
end;

procedure TForm1.Button1Click(Sender: TObject);

begin
 DisplayArray(1);
end;

[jaenicke]

Theoretisch geht es auch so:

Delphi-Quellcode:

  private
    type
      TLabelMode = (lmCustomerNumber, lmArticleNumber, lmBillNumber, lmOrderNumber, lmName);
    procedure SetSearchLabels(const ALabelMode: TLabelMode);

// ...

procedure TTblLieferanten.SetSearchLabels(const ALabelMode: TLabelMode);
const
  LabelCaptions: array[TLabelMode, 0..3] of string = (
    ('1 = Name', '2 = Kunden Nr.', '', ''),
    ('1 = Name', '3 = Artikel Nr.', '6 = Bezeichnung', '7 = Lieferanten'),
    ('2 = Kunden Nr.', '5 = Rechnungs Nr.', '', ''),
    ('1 = Name', '2 = Kunden Nr.', '4 = Auftrags Nr.', ''),
    ('1 = Name', '', '', '')
  );
  ComboContents: array[TLabelMode] of string = (
    '1'#13#10'2', '1'#13#10'3'#13#10'6'#13#10'7', '2'#13#10'5', '1'#13#10'2'#13#10'4', '1'
  );
begin
  Label10.Caption := LabelCaptions[ALabelMode][0];
  Label11.Caption := LabelCaptions[ALabelMode][1];
  Label13.Caption := LabelCaptions[ALabelMode][2];
  Label14.Caption := LabelCaptions[ALabelMode][3];
  ComboBox2.Items.Text := ComboContents[ALabelMode];
  ComboBox2.ItemIndex := 0;
end;

procedure TTblLieferanten.ComboBox3Change(Sender: TObject);
begin
  setSearchLabels(TLabelMode(ComboBox3.ItemIndex));
end;

Besser wäre es aber erstens die Komponenten ordentlich zu bezeichnen statt so ein Messie-Formular zu benutzen.

Und zweitens wäre es sinnvoller die Bezeichnungen usw. mit der Funktionalität kombiniert in eigene Klassen zu stecken. Dan können die verschiedenen Suchklassen von einer Elternklasse abgeleitet werden und es muss nur die entsprechende abgeleitete Klasse jeweils benutzt werden. Das macht es sehr viel übersichtlicher.

// EDIT:
Wo war der rote Kasten? Da steht ja schon eine ähnliche Lösung (die ich allerdings für unnötig unübersichtlich halte).

[Bummi]

@Jaenicke

Deines ist weniger "kompliziert", wenn die Anforderung kommt direkt über die Beschreibung auszuwählen, also Commbobox oder Radiogroup ohne Lables und sichtbare Zahlen, ist mein Ansatz schneller umgestrickt...

Delphi-Quellcode:

procedure TForm1.DisplayArray2(Nr:Integer);
var
  i:Integer;
begin
  Combobox1.Items.Clear;
  for I := 1 to 4 do
      begin
            if InfoRecArray[Nr,i].Nr>-1 then
              Combobox1.Items.AddObject(InfoRecArray[Nr,i].Caption,TObject( InfoRecArray[Nr,i].Nr));

      end;
   Combobox1.ItemIndex := 0;
end;

Der Zugriff auf die Nummer würde dann per Beispiel

Delphi-Quellcode:

Showmessage(IntToStr(Integer(Combobox1.Items.objects[Combobox1.ItemIndex])));

erfolgen ...

[jaenicke]

Um Metainformationen zu den Items einer ComboBox zu speichern, benutze ich aber lieber generische Ansätze.

Sei es durch einen class helper, der das generisch macht, oder durch ein Dictionary, über das die Informationen zu den visuellen Komponenten abrufbar sind.