Logisches problem

[SaFu]

Hallo

Hab gerade ein Problem und komm einfach nicht drauf.

Und zwar bei meinen Abfragen hier unten.
Mein Problem habe ich mit der Combobox4, wenn der Name in den Items vorhanden ist soll der eingebene Name nicht in die Items übernommen werden und es soll bei der 1. weitergehen und wenn der name nicht vorhanden ist soll er übernommen werden und es soll ebenfalls bei der 1.weitergehen
Kann mir da mal jemand helfen habe gerade ein Denk Problen

Delphi-Quellcode:

procedure TForm7.Button2Click(Sender: TObject);
var x,y:integer;
begin
if ComboBox4.Text <> '' then
 begin
   if ComboBox3.Items.IndexOf(ComboBox4.text)> -1 Then
  begin
   Application.MessageBox('    Vertretungsplan Existiert ','Speicherinformation',48);
     begin // bis hierhin funkioniert es wunderbar
      if not ComboBox4.Items.IndexOf(ComboBox4.Text)> -1 Then
        begin
         Combobox4.Items.Add(ComboBox4.Text);
        end;
         end
         else
         begin //1.
          Combobox3.Items.Add(ComboBox4.Text);// Text in die ComboBox3 List übergeben
          Vertrpl:= TStringlist.Create;   // Stringliste erzeugen
          Vertrpl.Add(IntToStr(StringGrid1.Colcount));
          Vertrpl.Add(IntToStr(StringGrid1.Rowcount));
          for x:= 0 to StringGrid1.ColCount-1 do
          for y:= 0 to StringGrid1.RowCount-1 do
          Vertrpl.Add(StringGrid1.Cells[x,y]);
          for x:= 0 to StringGrid1.ColCount-1 do
          Vertrpl.Add(IntToStr(StringGrid1.ColWidths[x]));
          for x:= 0 to StringGrid1.RowCount-1 do
          Vertrpl.Add(IntToStr(StringGrid1.RowHeights[x]));
          Vertrpl.Add(IntToStr(StringGrid1.Clientwidth));
          Vertrpl.Add(IntToStr(StringGrid1.Clientheight));
          Vertrpl.Add(IntToStr(Ord(StringGrid1.ScrollBars)));
          plan:= ExtractFilePath(ParamStr(0))+'Vertretungspläne\'+ ComboBox4.Text +'.txt';
          Vertrpl.SaveToFile(plan);
          Showmessage('Datei wurde in:' + sLineBreak + plan + ' gespeichert');
          Vertrpl.Free;
         end;
          begin
           lehrerl:= ExtractFilePath(ParamStr(0)) +'\Lehrer Liste\'+ 'liste.txt';
           ComboBox3.Items.SaveToFile(lehrerl);
           Form8.ListBox1.Clear;
           lehrerl2:= ExtractFilePath(ParamStr(0)) +'\Lehrer Liste\'+ 'alleLehrer.txt';
           Combobox4.Items.SaveToFile(lehrerl2);
           lehrerl:= ExtractFilePath(ParamStr(0)) +'\Lehrer Liste\'+ 'liste.txt';
           Form8.ListBox1.Items.LoadFromFile(lehrerl);
           Button7.click;
          end;
          end;
        end
     else
    Application.MessageBox('Bitte Name eingeben','Speicherinformation',48);
end;

Danke sascha

[Jürgen Thomas]

Hallo Sascha,

ich weigere mich, so einen Quelltext zu untersuchen. Korrigiere doch bitte alle Einrückungen, sodass jedes begin am Anfang eines Teilstücks steht, der darin enthaltene Text exakt 2 Zeichen eingerückt ist und jedes end exakt unter dem dazugehörigen begin steht. Genauso sollten alle if-then-else sauber positioniert werden.

Vielleicht fällt Dir dann ja selbst schon ein Fehler auf. Jürgen

[Nils_13]

Zitat von Jürgen Thomas:

Hallo Sascha,

ich weigere mich, so einen Quelltext zu untersuchen. Korrigiere doch bitte alle Einrückungen, sodass jedes begin am Anfang eines Teilstücks steht, der darin enthaltene Text exakt 2 Zeichen eingerückt ist und jedes end exakt unter dem dazugehörigen begin steht. Genauso sollten alle if-then-else sauber positioniert werden.

Vielleicht fällt Dir dann ja selbst schon ein Fehler auf. Jürgen

...wie wäre es damit die Komponenten richtig zu benennen ?

[mkinzler]

Das End an Zeile 13 scheint überflüssig zu sein.

[SaFu]

wie wäre es damit die Komponenten richtig zu benennen ? Was meinst du damit

[Der_Unwissende]

Zitat von fuknersascha:

Hab gerade ein Problem und komm einfach nicht drauf.

Hi,
das eigentliche Problem ist kein Logisches! Sorry, aber ich glaube dein eigentliches Problem ist deine Code-Formatierung. Die solltest Du mal ganz dringend überdenken! Sehe ich das richtig, dass Du einzelne begin end; Blöcke verwendest, die nicht weiter zu einer Bedingung/Strucktur gehören? Warum?
Dann solltest Du auch tunlichst die begins und ends korrekt einrücken. An sich solltest Du die 1. auch ruhig in eine eigene Prozedur auslagern. Erhöht einfach die Lesbarkeit!

Ja, für die Lesbarkeit solltest Du immer Einiges tun! Gute Lesbarkeit hilft Fehler zu vermeiden und sollten doch welche drin sein, findest Du die leichter. Form7, Button2, ComboBox4, ... keine guten Namen.

Versuche es mal so:

Delphi-Quellcode:

procedure TForm7.Button2Click(Sender: TObject);
var x,y:integer;
begin
  if ComboBox4.Text <> '' then
   begin
     if ComboBox3.Items.IndexOf(ComboBox4.text)> -1 Then
     begin
       Application.MessageBox('    Vertretungsplan Existiert ','Speicherinformation',48);

       if not ComboBox4.Items.IndexOf(ComboBox4.Text) > -1 Then
       begin
         Combobox4.Items.Add(ComboBox4.Text);
       end; // if not ComboBox4.Items.IndexOf(ComboBox4.Text) > -1

       //1.
       Combobox3.Items.Add(ComboBox4.Text);// Text in die ComboBox3 List übergeben
       Vertrpl:= TStringlist.Create;   // Stringliste erzeugen
       Vertrpl.Add(IntToStr(StringGrid1.Colcount));
       Vertrpl.Add(IntToStr(StringGrid1.Rowcount));
       
       for x:= 0 to StringGrid1.ColCount-1 do
       begin
         for y:= 0 to StringGrid1.RowCount-1 do
         begin
           Vertrpl.Add(StringGrid1.Cells[x,y]);
         end; // for y:= 0 to StringGrid1.RowCount-1 

         Vertrpl.Add(IntToStr(StringGrid1.ColWidths[x]));
         Vertrpl.Add(IntToStr(StringGrid1.RowHeights[x]));
       end; // for x:= 0 to StringGrid1.ColCount-1
          
       Vertrpl.Add(IntToStr(StringGrid1.Clientwidth));
       Vertrpl.Add(IntToStr(StringGrid1.Clientheight));
       Vertrpl.Add(IntToStr(Ord(StringGrid1.ScrollBars)));
       plan:= ExtractFilePath(ParamStr(0))+'Vertretungspläne\'+ ComboBox4.Text +'.txt';
       Vertrpl.SaveToFile(plan);
       Showmessage('Datei wurde in:' + sLineBreak + plan + ' gespeichert');
       Vertrpl.Free;
                   
       lehrerl:= ExtractFilePath(ParamStr(0)) +'\Lehrer Liste\'+ 'liste.txt';
       ComboBox3.Items.SaveToFile(lehrerl);
       Form8.ListBox1.Clear;
       lehrerl2:= ExtractFilePath(ParamStr(0)) +'\Lehrer Liste\'+ 'alleLehrer.txt';
       Combobox4.Items.SaveToFile(lehrerl2);
       lehrerl:= ExtractFilePath(ParamStr(0)) +'\Lehrer Liste\'+ 'liste.txt';
       Form8.ListBox1.Items.LoadFromFile(lehrerl);
       Button7.click;
     end; // if ComboBox3.Items.IndexOf(ComboBox4.text)> -1
  end // if ComboBox4.Text <> ''

  else
  begin
    Application.MessageBox('Bitte Name eingeben','Speicherinformation',48);
  end;
end;

Ich hoffe Du siehst hier schon einen Fortschritt in der Lesbarkeit. Das ist aber nur ein Anfang! Du solltest wie gesagt noch mehr kleine Prozeduren (mit sprechendem Namen!) einführen, so dass man schnell siehst, was genau gemacht wird.

Gruß Der Unwissende

[edit]
Wo war der rote Kasten?!
[/edit]

[SaFu]

Danke für diesen Tip.

Also muss euch mal sagen das ich es gut finde das ihr mir diese Tips gibt. daraus Kann man nur lernen. Aber das problem liegt einfach daran das ich:

1. Noch nicht so gut bin in Programmierung ( mach das erst seit 1 jahr)
2. Mir das nie richtig gezeigt wurde

Deshalb bin ich froh über jeden Tip denn ich bekomme. mir ist das ja auchunangenehm.

ich bitte euch etwas rücksicht zunehmen Danke

Gruss Sascha

P.S. man erkent einen Grossen Unterschied

[marabu]

Hallo Sascha,

das hier sieht seltsam aus:

Delphi-Quellcode:

// ...
      if not ComboBox4.Items.IndexOf(ComboBox4.Text)> -1 Then
        begin
         Combobox4.Items.Add(ComboBox4.Text);
        end;
// ...

So wird ein Eintrag nur dann in die Liste aufgenommen, wenn er bereits in der Liste ist. Macht das Sinn?

Um den Hinweis von Jürgen doch noch aufzugreifen: Programmieren ist soviel mehr als nur ein paar Komponenten auf eine Form zu ziehen und einen Quelltext ohne Fehlermeldungen zu übersetzen. Du wirst bereits nach einer Woche Urlaub deinen eigenen Quelltext nicht mehr verstehen, wenn du keine sinnvollen (sprechenden) Bezeichner verwendest - TForm1 und ähnliches sind Notlösungen, die von der IDE verwendet werden, weil sie es nicht besser weiß. Du aber kannst einen Button PrintButton nennen und eine Form MainForm oder so ähnlich. Solange du nur Zweizeiler postest wird sich niemand an einer fehlenden Namenskonvention stören, aber ein Code-Auszug mit 10 Komponenten und 30 Zeilen ist dann kaum noch in vernünftiger Zeit zu analysieren.

Freundliche Grüße

PS: Bin ich ein Opfer der Zeitdilatation? Da wurden doch glatt fünf Beiträge eingestellt, während ich meinen Beitrag zurecht gefeilt habe...

[SaFu]

Was war damit gemeint

Zitat:

Wo war der rote Kasten?!

Zitat:

Marabu

das hier sieht seltsam aus:

Delphi-Quellcode:

if not ComboBox4.Items.IndexOf(ComboBox4.Text)> -1 Then 
        begin 
         Combobox4.Items.Add(ComboBox4.Text); 
        end;

Ja soll natürlich nicht so sein


ja hätte es von Anfang an richtig beschreiben sollen.

Aber ich glaube #6 ist auch nicht so ganz richtig aber es ist aufjedenfall lesbarer

[xZise]

Hi Sascha...

Mir sind zwei sachen aufgefallen!

1. Du siehst nicht sofort, was wann und wie oft aufgerufen wird!
   
   Delphi-Quellcode:

   for x:= 0 to StringGrid1.ColCount-1 do
   for y:= 0 to StringGrid1.RowCount-1 do
   Vertrpl.Add(StringGrid1.Cells[x,y]);
   for x:= 0 to StringGrid1.ColCount-1 do
   Vertrpl.Add(IntToStr(StringGrid1.ColWidths[x]));
   for x:= 0 to StringGrid1.RowCount-1 do
   Vertrpl.Add(IntToStr(StringGrid1.RowHeights[x]));

   Deshalb sollte man immer schön einrücken:
   
   Delphi-Quellcode:

   for x:= 0 to StringGrid1.ColCount-1 do
     for y:= 0 to StringGrid1.RowCount-1 do
       Vertrpl.Add(StringGrid1.Cells[x,y]);  // Dieser Code wird von den beiden for-Schleifen ausgeführt
   for x:= 0 to StringGrid1.ColCount-1 do
     Vertrpl.Add(IntToStr(StringGrid1.ColWidths[x]));
   for x:= 0 to StringGrid1.RowCount-1 do
     Vertrpl.Add(IntToStr(StringGrid1.RowHeights[x]));

2. wenn man if not myInt = 1 then macht, dann ist es imho so, dass er nicht überprüft ob "myInt" nicht 1 ist.
   Deshalb hier entweder den Vergleichsoperator negieren, oder mit Klammern arbeiten:
   if myInt <> 1 then if not(myInt = 1) then

PS: Die Einrückung beeinflusst nicht die Exe Also kann man einrück wie man will, solange es leserlich bleibt.

Zitat von Jürgen Thomas:

ich weigere mich, so einen Quelltext zu untersuchen. Korrigiere doch bitte alle Einrückungen, sodass jedes begin am Anfang eines Teilstücks steht, der darin enthaltene Text exakt 2 Zeichen eingerückt ist und jedes end exakt unter dem dazugehörigen begin steht. Genauso sollten alle if-then-else sauber positioniert werden.

Du hast zwar recht, aber es bleibt jeden selber überlassen wie ein Einrückt!
Ich habe auch oft "if ... then begin" geschrieben, und das "end" auf Höhe von "if" geschrieben

PS: Es stimmt schon dass der Code sehr unleserlich ist ^^

Zitat von fuknersascha:

Was war damit gemeint

Zitat:

Wo war der rote Kasten?!

Unten in der "Postchronik", werden Posts, die während des schreibens hinzukommen rot markiert (jedenfalls sollten sie es ^^)

Zitat von fuknersascha:

Aber ich glaube #6 ist auch nicht so ganz richtig aber es ist aufjedenfall lesbarer

Editiere doch den Eingangspost

[edit]Wenn ich es richtig verstanden habe, dann überprüft ob "IndexOf(ComboBox4.Text)" "-1" ist. Wenn es so ist, dann kannst du das machen, was gemacht werden soll, wenn ein Eintrag nicht existiert. Ansonsten das andere ausführen ^^[/edit]