Hilfe beim Quelletext kürzen

[Langa]

Brauch mal wieder hilfe

wie kann ich den quelltext kürzen aus dünnen oder geht das nicht??? und so ist schon das kürzeste???

Delphi-Quellcode:

procedure TForm1.Button2Click(Sender: TObject);
begin

        case form1.ComboBox1.ItemIndex of
           0 : form1.Timer1.Interval := 60;
           1 : form1.Timer1.Interval := 30;
           2 : form1.Timer1.Interval := 15;
        end;
   form1.button2.Visible:=false;
   form1.Button4.Visible:=true;

           if winkel>360 then winkel:=0;

if IsNotInt(edit1.Text) or IsNotInt(edit2.Text) then begin
    application.MessageBox(pchar('Bitte geben sie nur Zahlen ein!!!'), '!!! Achtung !!!', MB_OK);   // ausgabe einer nachricht wenn im feld keine zahlt steht
    exit;
  end else Begin
        r1:=strtoint(edit1.text);
        r2:=strtoint(edit2.Text);                                                  //umwandeln des des string aus den eingabefeldern in einen integer wert und speichern auf dem integer l und r
  end;
  z:=r1+2*r2;
  if z>200 then begin
  if MessageDlg('soll wirklich gemalt weden???',mtInformation , [mbYes, mbNo], 0) = mrYes then begin
      with bitmap1.Canvas do begin
      ellipse(x0-r1,y0-r1,x0+r1,y0+r1);            //zentraler Kreis
      ellipse(x0+r1,y0+r2,x0+r1+2*r2,y0-r2);       //außen Kreis
      pen.Color:=clred;
      ellipse(x0+r1+2,y0+2,x0+r1-2,y0-2);
      pen.Color:=clblack;
      end;
      timer1.Enabled:=true;
      end else begin
      form1.Edit1.text:='50';
      form1.Edit2.Text:='10';
      form1.button2.Visible:=true;
      form1.Button4.Visible:=false;
      end;
      end else begin
      with bitmap1.Canvas do begin
      ellipse(x0-r1,y0-r1,x0+r1,y0+r1);            //zentraler Kreis
      ellipse(x0+r1,y0+r2,x0+r1+2*r2,y0-r2);       //außen Kreis
      pen.Color:=clred;
      ellipse(x0+r1+2,y0+2,x0+r1-2,y0-2);
      pen.Color:=clblack;
      end;
      timer1.Enabled:=true;
      end;
      paintbox1.Canvas.Draw(0,0,bitmap1);


end;

[mkinzler]

Das form1 solltest du weglassen, da form1 die Bezeichnug einer Instanz von TForm1 ist.

[Nikolas]

Es ist etwas schwierig, sich in deinem Code zurecht zu finden, da du überhaupt nicht einrückst und nicht klar ist, welches begin zu welchem end gehört. Als Fausregel gilt eigentlich, dass zusammengehörenende Begin-End-paare immer untereinander stehen und nicht wie bei dir im Text:

if z>200 then begin
if MessageDlg('soll wirklich gemalt weden???',mtInformation , [mbYes, mbNo], 0) = mrYes then begin
with bitmap1.Canvas do begin
ellipse(x0-r1,y0-r1,x0+r1,y0+r1); //zentraler Kreis
ellipse(x0+r1,y0+r2,x0+r1+2*r2,y0-r2); //außen Kreis
pen.Color:=clred;
ellipse(x0+r1+2,y0+2,x0+r1-2,y0-2);
pen.Color:=clblack;
end;
timer1.Enabled:=true;
end else begin

[himitsu]

Zitat:

Zitat von mkinzler

Das form1 solltest du weglassen, da form1 die Bezeichnug einer Instanz von TForm1 ist.

Das ist wohl war, wenn du es unbedingt angeben willst, dann bitte auch konsequent ... beim Timer und der PaintBox ist es ja auch nicht da.

Dann könne etwas mehr mit Einrücken gemacht werden und dann auch noch etwas gleichmäßiger, ist ja schon fast unübersichtlich ;)

Wenn die ComboBox1 nur die 3 Werte hat, könntest du ja auch ein Array verwenden:
Const IntervalArray: Array[0..2] of Integer = (60, 30, 15);

Delphi-Quellcode:

procedure TForm1.Button2Click(Sender: TObject);
begin 
      Timer1.Interval := IntervalArray[ComboBox1.ItemIndex];
      ...

[markusj]

Das

Delphi-Quellcode:

pchar('Bitte geben sie nur Zahlen ein!!!')

kann man imho einfach Weglassen. Zumindest mache ich das so.

mfG

Markus

[glkgereon]

Zitat:

Zitat von himitsu

Wenn die ComboBox1 nur die 3 Werte hat, könntest du ja auch ein Array verwenden:
Const IntervalArray: Array[0..2] of Integer = (60, 30, 15);

Delphi-Quellcode:

procedure TForm1.Button2Click(Sender: TObject);
begin 
      Timer1.Interval := IntervalArray[ComboBox1.ItemIndex];
      ...

oder so: :mrgreen:

Delphi-Quellcode:

Timer1.Interval := 60 div IntPower(2,ComboBox1.ItemIndex);

ansonsten ist da glaub ich nicht viel zu kürzen...

[GimbaR]

Ich hab mir mal die Freiheit genommen und den Quelltext mal nach meinen Vorstellungen formatiert, um deine Frage zu beantworten.

Delphi-Quellcode:

procedure TForm1.Button2Click(Sender: TObject); 
begin 
  case ComboBox1.ItemIndex of 
    0 : Timer1.Interval := 60; 
    1 : Timer1.Interval := 30; 
    2 : Timer1.Interval := 15; 
  end; 
   button2.Visible:= false; 
   Button4.Visible:= true; 

  if winkel > 360
    then winkel:=0; 

  if IsNotInt(Edit1.Text) or IsNotInt(Edit2.Text) then
  begin 
    MessageBox(pchar('Bitte geben sie nur Zahlen ein!!!'), '!!! Achtung !!!', MB_OK); // ausgabe einer nachricht wenn im feld keine zahlt steht 
    exit; 
  end else
  begin 
    r1:=strtoint(Edit1.text); 
    r2:=strtoint(Edit2.Text); //umwandeln des des string aus den eingabefeldern in einen integer wert und speichern auf dem integer l und r 
  end; 
  
  z:= r1 + 2 * r2; 
  
  if z > 200 then
  begin 
    if MessageDlg('soll wirklich gemalt weden???',mtInformation , [mbYes, mbNo], 0) = mrYes then
    begin 
      with Bitmap1.Canvas do
      begin 
        Ellipse(x0 - r1, y0 - r1, x0 + r1, y0 + r1);            //zentraler Kreis 
        Ellipse(x0 + r1, y0 + r2, x0 + r1 + 2 * r2, y0 - r2);   //außen Kreis 
        Pen.Color:= clred; 
        Ellipse(x0 + r1 + 2, y0 + 2, x0 + r1 - 2, y0 - 2); 
        Pen.Color:= clblack; 
      end;       
      Timer1.Enabled:= true; 
      end else
      begin 
        Edit1.text:= '50'; 
        Edit2.Text:= '10'; 
        button2.Visible:= true; 
        Button4.Visible:= false; 
      end; 
    end else
    begin 
      with Bitmap1.Canvas do begin 
        Ellipse(x0 - r1, y0 - r1, x0 + r1, y0 + r1);            //zentraler Kreis
        Ellipse(x0 + r1, y0 + r2, x0 + r1 + 2 * r2, y0 - r2);   //außen Kreis
        Pen.Color:= clred;
        Ellipse(x0 + r1 + 2, y0 + 2, x0 + r1 - 2, y0 - 2);
        Pen.Color:= clblack;
      end; 
      Timer1.Enabled:=true; 
   end; 
   
   Paintbox1.Canvas.Draw(0, 0, Bitmap1); 
end;

Aber ehrlich gesagt, wüsste ich nicht was man da kürzen sollte :gruebel:
Das einzige, was mir aufgefallen sind die Zeilen:

Delphi-Quellcode:

 
  if IsNotInt(Edit1.Text) or IsNotInt(Edit2.Text) then
  begin 
    MessageBox(pchar('Bitte geben sie nur Zahlen ein!!!'), '!!! Achtung !!!', MB_OK); // ausgabe einer nachricht wenn im feld keine zahlt steht 
    exit; 
  end else

Da hier ja des öfteren über das "exit" gelästert wurde ( :wink: ), wäre es sinnvoller diese Zeilen ganz am Anfang zu schrieben, quasi:

Delphi-Quellcode:

procedure TForm1.Button2Click(Sender: TObject); 
begin   
  if IsNotInt(Edit1.Text) or IsNotInt(Edit2.Text) then
  begin 
    MessageBox(pchar('Bitte geben sie nur Zahlen ein!!!'), '!!! Achtung !!!', MB_OK); // ausgabe einer nachricht wenn im feld keine zahlt steht 
  end else
  begin
    case ComboBox1.ItemIndex of 
    .
    .
    .
  end;
end.

[mkinzler]

Zitat:

Zitat von himitsu

Das ist wohl war, wenn du es unbedingt angeben willst, dann bitte auch konsequent ... beim Timer und der PaintBox ist es ja auch nicht da.

Dann aber als TForm1 und nicht als form1

[alzaimar]

Wird zwar nicht kürzer, aber dafür übersichtlicher:

Gib den Komponenten sinnvolle Namen. Statt 'Edit1' wäre z.B. edZahl besser, dann weiss man gleich, was da eigentlich rein soll.

Funktionen sollten nicht prüfen, ob etwas NICHT zutrifft, sondern, ob etwas zutrifft. Statt 'IsNotInt' sollte man die Funktion in 'IsInteger' umbenennen und das Resultat negieren ("Sind sie sich nicht sicher, das sie keine Zahl nicht eingeben wollen?")

[Hawkeye219]

Darf ich mitspielen?

Delphi-Quellcode:

procedure TForm1.Button2Click (Sender: TObject);
begin

  case ComboBox1.ItemIndex of
    0 : Timer1.Interval := 60;
    1 : Timer1.Interval := 30;
    2 : Timer1.Interval := 15;
  end;

  Button2.Visible := false;
  Button4.Visible := true;

  if winkel > 360 then
    winkel := 0;

  if TryStrToInt(edit1.Text, r1) and TryStrToInt(edit2.Text, r2) then
    begin

      z := r1 + 2 * r2;

      if (z > 200) and (MessageDlg('Soll wirklich gemalt werden?', mtInformation , [mbYes, mbNo], 0) <> mrYes) then
        begin
          Edit1.text := '50';
          Edit2.Text := '10';
          Button2.Visible := true;
          Button4.Visible := false;
        end
      else
        begin
          with Bitmap1.Canvas do
            begin
              Ellipse (x0 - r1, y0 - r1 ,x0 + r1, y0 + r1);           // zentraler Kreis
              Ellipse (x0 + r1, y0 + r2 ,x0 + r1 + 2 * r2, y0 - r2);  // außen Kreis
              Pen.Color := clred;
              Ellipse(x0 + r1 + 2, y0 + 2, x0 + r1 - 2, y0 - 2);
              Pen.Color := clblack;
            end;
          timer1.Enabled := true;
        end;

      paintbox1.Canvas.Draw (0, 0, bitmap1);

    end
  else
    // ausgabe einer nachricht wenn im feld keine zahlt steht
    application.MessageBox(pchar('Bitte geben sie nur Zahlen ein!'), '!!! Achtung !!!', MB_OK);

end;

Gruß Hawkeye

[Langa]

naja schön und gut Hawkeye nur wenn ich den quelltext von die nehme ist das problem das der vorher schon rummekert das es kein integer ist und dann erst die message aus gibt.
das sollte ja gerade nicht sein er soll ja nur die nachricht zeigen

[Hawkeye219]

Hallo Langa,

wo meckert er rum? :gruebel:
TryStrInt versucht den übergebenen String in einen Integer-Wert umzuwandeln und liefert im Fehlerfall False zurück. Falls also in einem oder beiden Edit-Feldern keine Zahl steht, sollte lediglich die MessageBox erscheinen.

Gruß Hawkeye

[Langa]

wenn ich das in den reset button rein mache gehts aber nich beim start button das sagt der erst es ist kein integer und dann erst kommt die messagebox von meinem programm

[Luckie]

Wie wäre es mit einer etwas verschwenderischeren Interpunktion? Deine Sätze kann man ja kaum lesen oder verstehen ohne Punkt und Komma.

[sk.Silvia]

thats my syntax...do you like it?

Delphi-Quellcode:

procedure TForm1.Button2Click (Sender: TObject);
  begin

  case ComboBox1.ItemIndex of
    0:Timer1.Interval:=60;
    1:Timer1.Interval:=30;
    2:Timer1.Interval:=15;
    end;

  Button2.Visible:=false;
  Button4.Visible:=true;

  if winkel>360 then winkel:=0;

  if TryStrToInt(edit1.Text, r1) and TryStrToInt(edit2.Text, r2) then
      begin

      z:=r1+2*r2;

      if (z>200) and (MessageDlg('Soll wirklich gemalt werden?', mtInformation , [mbYes, mbNo], 0) <> mrYes) then
          begin
          Edit1.text:='50';
          Edit2.Text:='10';
          Button2.Visible:=true;
          Button4.Visible:=false;
          end
      else
          begin
          with Bitmap1.Canvas do
              begin
              Ellipse(x0-r1,y0-r1,x0+r1,y0+r1);           // zentraler Kreis
              Ellipse(x0+r1,y0+r2,x0+r1+2*r2,y0-r2);  // außen Kreis
              Pen.Color:=clred;
              Ellipse(x0+r1+2,y0+2,x0+r1-2,y0-2);
              Pen.Color := clblack;
              end;
          timer1.Enabled:=true;
          end;

      paintbox1.Canvas.Draw (0,0,bitmap1);
      end
  else application.MessageBox(pchar('Bitte geben sie nur Zahlen ein!'), '!!! Achtung !!!', MB_OK);

  end;

[marabu]

Nur eine Detailstudie:

Delphi-Quellcode:

{
case ComboBox1.ItemIndex of
  0: Timer1.Interval:=60;
  1: Timer1.Interval:=30;
  2: Timer1.Interval:=15;
end;
}

with ComboBox1, Timer1 do
  Interval := 60 shr ItemIndex;

Und noch eine:

Delphi-Quellcode:

{
if winkel>360 then winkel:=0;
}
winkel := winkel mod 360;

Grüße vom marabu