guter stil????

[Mr. Pink]

hallo,
auch wenn ich noch vor ein paar stunden eine frage zu diesem programm gestellt habe, hab ich doch jetzt eine ganz andere:
Ich möchte mal hören, ob mein schreibstil in delphi gut oder schlecht ist. ich kam drauf, indem ich mir mal andere sources angeschaut habe, die irgendwie ganz anders aufgebaut sind als meine codes!

Ich würd mich freuen wenn sich mal jemand den code angucken und sagen könnte was ich besser/anders machen sollte.

danke an alle die sich die mühe machen!!!
btw, das ganze ist zur primfaktorzerlegung gedacht.

Delphi-Quellcode:

unit Unit1;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, StdCtrls, ExtCtrls;

type
  TForm1 = class(TForm)
    Label1: TLabel;
    Edit1: TEdit;
    Button1: TButton;
    Label2: TLabel;
    procedure Button1Click(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var m:int64;
  Form1: TForm1;

implementation

{$R *.dfm}

function pruefen(n:int64):boolean;
var x:int64;
    p:boolean;
begin
 p:=true;
 x:=round(n/2);
 if n>=2 then
 begin
  while x>1 do
  begin
   if (n mod x) = 0 then
   begin
    p:=false;
    break;
   end;
   x:=x-1;
  end;
 end
 else p:=false;
 if p=true then pruefen:=true
 else pruefen:=false;
end;

procedure zerlegen(n:int64);
var x:int64;
begin
 x:=2;
 while (n mod x<>0) do x:=x+1;
 if length(form1.Label2.Caption)>0 then form1.Label2.Caption:=form1.Label2.Caption+'*';
 form1.Label2.Caption:=form1.Label2.Caption+inttostr(x);
 if pruefen(n div x)=false then zerlegen(n div x);
 m:=m*x;
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
 m:=1;
 label2.Caption:='';
 if pruefen(strtoint(edit1.Text))=true then label2.Caption:=edit1.Text+' ist eine Primzahl!'
 else
 begin
  zerlegen(strtoint(edit1.Text));
  label2.Caption:=label2.Caption+'*'+inttostr(strtoint(edit1.Text)div m);
 end;
end;

end.

[GLaforge]

Hi,
was mir gleich auffällt sind fehlende Kommentare, sind für dich und uns schon meist von nutzen. Desweiteren würde ich eine Einrückung bei den if Konstrukten vornehmen. Ich schreibe immer das else unter das then etc. Ausserdem würde ich nach dem 1. Begin die fortlaufenden Zeilen auch um ein bis zwei Leerzeichen einrücken.
Jedoch muss man auch sagen, dass es so keine wirklichen Festlegungen gibt. Es sollte aber immer dein Ziel sein, dass du und jeder andere sich im QT zurechtfindet, eben indem man Schleifen etc. gesondert einrückt.

[Matze]

Nur ein Wort zur Formatierung, die ist so ja schrecklich.
Inhaltlich leicht verändert (wichtig!) und wenn du so Wert auf Ordentlichkeit legst, solltest du den Code auch entsprechend formatieren. Ich würde ihn in deinem Fall so formatieren:

Delphi-Quellcode:

unit Unit1;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, StdCtrls, ExtCtrls;

type
  TForm1 = class(TForm)
    Label1: TLabel;
    Edit1: TEdit;
    Button1: TButton;
    Label2: TLabel;
    procedure Button1Click(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var 
  m: Int64;
  Form1: TForm1;

implementation

{$R *.dfm}

function pruefen(n: Int64): Boolean;
var 
  x: Int64;
  p: Boolean;
begin
  p := true;
  x := round(n / 2);
  if n >= 2 then
  begin
    while x > 1 do
    begin
      if (n mod x) = 0 then
      begin
        p := false;
        break;
      end;
      x := x - 1;
    end;
  end else 
    p := false;

  if p then               // überprüfe niemals auf "= true" !! 
    pruefen := true else  // Willst du die Gründe wissen -> Suchfunktion.   
      pruefen := false;   // 'p := pruefen' müsste hier auch gehen                                  
end;

procedure zerlegen(n: Int64);
var 
  x: Int64;
begin
  x := 2;
  while (n mod x <> 0) do 
    x := x + 1;

  if Length(Form1.Label2.Caption) > 0 then
    Form1.Label2.Caption := Form1.Label2.Caption + '*';
  Form1.Label2.Caption := Form1.Label2.Caption + IntToStr(x);

  if pruefen(n div x) = false then
    zerlegen(n div x);

  m := m * x;
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  m := 1;
  Label2.Caption := '';

  if pruefen(StrToInt(Edit1.Text)) then   // kein "= true"!
  begin
    Label2.Caption := Edit1.Text + ' ist eine Primzahl!' else
    begin
      zerlegen(StrToInt(Edit1.Text));
      Label2.Caption := Label2.Caption + '*' + 
        IntToStr(StrToInt(Edit1.Text) div m);
    end;
  end;
end;

end.

Bei Gelegenheit kannst du dir auch einmal den Borland-Styleguide durchlesen.

[3_of_8]

Mir fallen auf die Schnelle zwei Sachen auf:

1. Einrückungen. Manchmal vorhanden, manchmal nicht. Nimm nen Sourceformatter, google mal nach Delforex.
2. if aboolean=true then... ist nicht korrekt. if aboolean then... muss es heißen. Denn ersteres vergleicht den Wert von aboolean mit dem Wert der Konstante True. Zweiteres schaut nach, ob aboolean wahr ist oder nicht. Das macht vor allem dann einen Unterschied, wenn du mit LongBool oder ähnlichem arbeitest.

[Ultimator]

Delphi-Quellcode:

function pruefen(n: int64):boolean; 
var x: int64;  
begin 
  result := true; 
  x := round(n/2); 
  if n >= 2 then 
    begin 
      while x > 1 do 
        begin 
          if (n mod x) = 0 then 
            begin 
              result := false; 
              break; 
            end; 
         dec(x); 
       end; 
     end 
   else 
     result := false; 
end; 

procedure zerlegen(n:int64); 
var x: int64; 
begin 
  x := 2; 
  while (n mod x <> 0) do
    x := x+1; 
  if length(Form1.Label2.Caption) > 0 then
    Form1.Label2.Caption := Form1.Label2.Caption + '*'; 
  Form1.Label2.Caption := Form1.Label2.Caption + inttostr(x); 
  if not pruefen(n div x) then 
    zerlegen(n div x); 
  m := m*x; 
end; 

procedure TForm1.Button1Click(Sender: TObject); 
begin 
  m := 1; 
  Label2.Caption := ''; 
  if pruefen(strtoint(edit1.Text)) then 
    Label2.Caption := Edit1.Text + ' ist eine Primzahl!' 
  else 
    begin 
      zerlegen(strtoint(edit1.Text)); 
      Label2.Caption := Label2.Caption + '*' + inttostr(strtoint(edit1.Text) div m); 
    end; 
end;

So würd ichs machen.
Ich hab mir den Algo an sich nicht angeschaut, wie effektiv und richtig der ist ;)
Was hab ich gemacht: Oben in der zerlegen-Funktion hab ich die Variable p rausgenommen und gleich result verwendet. Dann hab ich alle Einrückungsebenen konsequent durchgezogen, Alle Anfangsbuchstaben von Objekten groß geschrieben, Leerzeichen vor und hinter jedes ":=" gesetzt und auch vor und hinter jedes "+" in den Stringzusammensetzungen.

//edit1: Zusätzlich noch hinter jedem ":" in den Variablendeklarationen und den Parameterangaben ein Leerzeichen eingefügt.
//edit2: Leerzeichen rund um die <, >, <=, >= und <>-Vergleiche eingefügt.

[Matze]

Zitat:

Zitat von Ultimator

So würd ichs machen.

Da fand ich seines ja fast übersichtlicher. :stupid:

Ich persönlich vorde vor begin nie einrücken. Aber darüber kann man sich natürlich streiten.

[xaromz]

Hallo,

ein paar Anmerkungen:
- mehr einrücken
- ab und zu mit Lehrzeilen logische Schritte trennen
- Kommentare
- vor und nach Operatoren ein Leerzeichen
- Zwischenergebnisse nicht mehrfach ausrechnen, sondern speichern
- statt "if a = False" lieber "if not a"
- bei Funktionen/Schlüsselwörtern Groß-/Kleinschreibung beachten
- Variablennamen evtl. aussagekräftiger

Beispiel:

Delphi-Quellcode:

procedure zerlegen(n:int64); 
var x:int64; 
begin 
x:=2; 
while (n mod x<>0) do x:=x+1; 
if length(form1.Label2.Caption)>0 then form1.Label2.Caption:=form1.Label2.Caption+'*'; 
form1.Label2.Caption:=form1.Label2.Caption+inttostr(x); 
if pruefen(n div x)=false then zerlegen(n div x); 
m:=m*x; 
end;

Besser:

Delphi-Quellcode:

procedure Zerlegen(n: Int64); 
var 
  x: Int64; 
  Zwischen: Int64;
begin 
  x := 2; 
  while (n mod x <> 0) do 
    Inc(x);

  if Length(Form1.Label2.Caption) > 0 then
    Form1.Label2.Caption := form1.Label2.Caption + '*'; 
  Form1.Label2.Caption := Form1.Label2.Caption + IntToStr(x);

  Zwischen := n div x;
  if not Pruefen(Zwischen) then 
    Zerlegen(Zwischen); 
  m := m * x; 
end;

Ansonsten schon nicht so schlecht. Insbeondere bei der Einrückung sehe ich Potential, auch wenn das bei Dir teilweise noch schwer lesbar ist.

Gruß
xaromz
//Edit: Wie viele Rechtschreibfehler man so produzieren kann...

[Ultimator]

Zitat:

Zitat von Matze

Zitat:

Zitat von Ultimator

So würd ichs machen.

Da fand ich seines ja fast übersichtlicher. :stupid:

Ich persönlich vorde vor begin nie einrücken. Aber darüber kann man sich natürlich streiten.

Hm, ich hab mir das so angewöhnt und finds auch gut zu lesen *g*

[Rastaman]

Also ich häts so gemacht (von der Formatierung, vom Inhalt weiß ich nicht)

Delphi-Quellcode:

unit Unit1;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, StdCtrls, ExtCtrls;

type
  TForm1 = class(TForm)
    Label1: TLabel;
    Edit1: TEdit;
    Button1: TButton;
    Label2: TLabel;
    procedure Button1Click(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var m:int64;
  Form1: TForm1;

implementation

{$R *.dfm}

function pruefen(n: int64): boolean;
var
  x: int64;
  p: boolean;
begin
  p := true;
  x := round(n / 2);
  if n >= 2 then
  begin
    while x > 1 do
    begin
      if (n mod x) = 0 then
      begin
        p := false;
        break;
      end;
      x := x-1;
    end;
  end else
    p := false;

  if p = true then
    pruefen := true
  else
    pruefen := false;
end;

procedure zerlegen(n: int64);
var
  x:int64;
begin
  x:=2;
  while (n mod x <> 0) do
    x := x + 1;

  if Length(form1.Label2.Caption) > 0 then
    Form1.Label2.Caption := Form1.Label2.Caption + '*';

  Form1.Label2.Caption := Form1.Label2.Caption + IntTosSr(x);
  if pruefen(n div x) = false
    then zerlegen(n div x);

  m := m * x;
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  m := 1;
  Label2.Caption := '';
  if pruefen(StrToInt(Edit1.Text)) = True then
    Label2.Caption := Edit1.Text + ' ist eine Primzahl!'
  else begin
    zerlegen(StrToInt(Edit1.Text));
    Label2.Caption := Label2.Caption + '*' + IntToStr(StrToInt(Edit1.Text) div m);
  end;
end;

end.

[xaromz]

Ach ja,
in der Routine "Pruefen":

Delphi-Quellcode:

  if p = true then 
    pruefen := true 
  else 
    pruefen := false;

Wie wäre es hier mit

Delphi-Quellcode:

Result := p;

Gruß
xaromz