guter stil????

[tigerman33]

Ist erstens relativ offensichtlich, der Fehler, und mir persönlich auch noch nicht passiert.

[sECuRE]

Hi,

Zitat von tigerman33:

kann ich genau wie Christian Seehase gerne verzichten. Auch den viel gepredigten begin-end Block um einzeilige then/else-Zweige oder Schleifen finde ich überflüssig. Das müllt mir bloß den Code zu.

Da stimme ich zu. Allerdings hab' ich noch etwas anzumerken:

Zitat von tigerman33:

Delphi-Quellcode:

while x > 0 do
  if x < 5 then
    inc(x) else
    dec(x);

Das else würde ich an den Zeilenanfang setzen, denn wenn man schnell über Code liest, und einen der ausgeführte Text der if-Bedingung nicht interessiert, liest man die Zeile für gewöhnlich nicht zu Ende - und übersieht eventuell das else.

Letztendlich würde ich es so machen:

Delphi-Quellcode:

// 1) const sorgt eventuell für schnellere Parameterweitergabe, sicher aber dafür,
// dass man sich nicht einfach Variablen überschreibt
// 2) ein normaler Integer langt und ist auf 32bit-CPUs schneller berechnet
// - StrToInt liefert auch nur einen Integer und keinen int64
// 3) Funktionsname:
// Man prüft bei der Verwendung von functions *generell* - Was wird also geprüft?
function IstPrim(const n:integer):boolean;
var x:integer;
begin
  // Sonderfall 1 beachten
  if (n = 1) then begin
    result:=false;
    exit;
  end;

  // Deine Primzahlprüfung hab' ich nicht ganz verstanden. Warum prüfst du nur die Teiler
  // bis Round(N / 2)? Normalerweise prüft man doch von 2 bis Sqrt(N)!
  result:=true;
  for x:=2 to Trunc(Sqrt(N)) do
    if ((n mod x) = 0) then begin
      result:=false;
      exit;
    end;
end;


// integer: siehe oben
procedure Zerlegen(var OutputLabel:TLabel; const n:integer); 
var x:integer; 
begin 
  x:=2; 
  while ((n mod x) <> 0) do 
    Inc(c);

  // Die Bedingung vorher war Schwachsinn (*sorry*) 
  OutputLabel.Caption:=OutputLabel.Caption+'*'+IntToStr(x); 

  if (not IstPrim(n div x)) then 
    Zerlegen(n div x); 

  m:=m*x; 
end; 

procedure TForm1.Button1Click(Sender: TObject); 
begin 
  m:=1; 
  label2.Caption:=''; 
  if (IstPrim(strtoint(edit1.Text))) then 
    Label2.Caption:=edit1.Text+' ist eine Primzahl!' 
  else begin 
    Zerlegen(Label2, StrToInt(Edit1.Text)); 
    Label2.Caption:=Label2.Caption+'*'+IntToStr(StrToInt(Edit1.Text) div m); 
  end; 
end;

Die globale Variable m hab' ich erst mal dringelassen, ich hab' ja schon genug anderes kritisiert .

cu

[negaH]

@Secure:

sorry aber

Delphi-Quellcode:

function IstPrim(const n:integer):boolean; 
var x:integer; 
begin 
  // Sonderfall 1 beachten 
  if (n = 1) then begin 
    result:=false; 
    exit; 
  end; 

  // Deine Primzahlprüfung hab' ich nicht ganz verstanden. Warum prüfst du nur die Teiler 
  // bis Round(N / 2)? Normalerweise prüft man doch von 2 bis Sqrt(N)! 
  result:=true; 
  for x:=2 to Trunc(Sqrt(N)) do 
    if ((n mod x) = 0) then begin 
      result:=false; 
      exit; 
    end; 
end;

halte ich fr ziemlich übel.

1.) Du vereinfachst durch eigene Annahmen die gewünschte Funktionsweise des Programmieres, indem du enifach mal davon ausgehst das man eh nur Zahlen bis 2^31 testen möchte, statt eben wie im Original mit Int64 zu rechnen. Das ist wohl sehr schlechter Stil. Die Aufgabe des programmieres ist es die Forderungen EXAKT umzusetzen.

2.) dein Source ist fehlerhaft. Was ist mit 0, oder -1 oder allen Zahlen bis -(2^31-1). Übergibt man deiner Funktion einen Integer im Wertebereich der Integer von -2^31 bis 2^31 dann macht sie defacto exakt 50% Fehlerhafte Ausagen.

3.) dein Schreibstil, hm naja. Das Besondere in PASCAL ist es das man Gorß/Klein Schreibung als Mittel zu besseren Lesbarkeit einsetzen kann. Du verzichtest darauf indem du alles klein schreibst. Damit verzichtest du auf Übersichtlickeit.

4.) N. Wirth hat mit Absicht die "Gültigkeit eines Source Blocks" visuell grafisch auch als eingerückter Block mit einem senkrecht untereinander stehendem BEGIN und END eingeführt. Es ist also essentiell das BEGIN und END auf gleicher Einrückungsebene zu schreiben.

5.) was nun ? IstPrim() oder IsPrime() ? aber nicht IsPrim() das ist denglish.

6.) in der Mathemtik ist es üblich N als Synonym für eine Natürlich Zahl zu betrachten, so wie R für rationelle Zahlen etc.pp.

7.) Sonderfall 1 ist zu wenig und berücksicht eben nicht negative Zahlen

8.) die Berechnung in for X := 0 to Trunc(Sqrt(n)) ist zwar auf Grund des Delphi Compilers möglich sollte aber aus Sicht eines Standard PASCAL Codes vermieden werden. Unter Umständen, bei erweiterte PASCAL Dialekten mit einer for i to x step y Syntax ist nämlich x als Schleifenendebedingung dynamisch veränderbar.

9.) wenn es geht sollte man immer auf unnötige Sprachkonstruke verzichten und statt dessen mit "mathematischen" Zuweisungen arbeiten.

10.) wenn es geht exit/goto/continue vermeinden

11.) man muß nur mit ungeraden Zahlen die Trialdivision durchführen. Da es aber in Delphi keine for i tox step y Schleife gibt müsste man sich behelfen mit for x := 2 to Trunc(Sqrt(N)) div 2 do und mit if N mod (i*2+1) = 0 arrbeiten, um eben den offensichtlichen mathematischen Erfordernissen gerecht zu werden. Es geht hier um Exaktheit in der Programmier Arbeit die man leistet. Allerdings ist eine solche Zähleschleife wesentlich schlechter zu verstehen als eine while Schleife. In deinem Falle testet deine Funktion die Zahl N doppelt so häufig als es nötig wäre, ergo Effizienzeinbußen.

Delphi-Quellcode:

function IsPrime(const N: Int64): Boolean;
var
  Root,Candidate: Int64;
begin
  Result := Odd(N) and (N > 2);  // Eine Primzahl ist ungerade und größer 1, ausser der 2
  if Result then
  begin
     // da wir N als Int64 vorgegeben haben können wir NICHT per Zählschleifen arbeiten, Delphi unterstützt
     // keine Int64 als Schleifenzähler, ergo while schleife
     Root := Trunc(Sqrt(N));
     Candidate := 3;
     while Result and (Candidate < Root) do
     begin
       Result := N mod Candidate <> 0;  // statt IF THEN eine mathematische, boolsche Zuweisung
       Candidate := Candidate + 2;
     end;      
  end else 
    Result := N = 2; // den einzigsten Fall einer geraden Primzahl berücksichtigen
                     // statt IF THEN eine boolsche Zuweisung 
end;

[Khabarakh]

Es muss
while Result and (Candidate <= Root) do statt
while Result and (Candidate < Root) do heißen, da ja schon mit Trunc abgerundet wurde.

[edit]In deiner ersten Funktion ist es noch richtig, da fehlt dafür ein Semikolon . [/edit]

[negaH]

Delphi-Quellcode:

while x > 0 do 
  if x < 5 then 
    inc(x) else 
    dec(x);

sowas ist ebenfalls schlecht, das es nicht mehr grafisch von link nach rechts erfassbar ist. Man muß also immer ganz genau lesen damit man weis was passiert.

Delphi-Quellcode:

while x > 0 do  
begin
  if x < 5 then
  begin
    Inc(x);
  end 
  else
  begin
    Dec(X);
  end; 
end;

das wäre Standard konform, ist aber ziemlich aufgebläht, also hat man auch im Standard Vereinfachungen der Schreibweise berücksichtigt. ABER das dann grafisch konform mit "virtuellen" Einrückungen a 2 Zeichen.

Delphi-Quellcode:

while x > 0 do
  if x < 5 then
  begin
    Inc(x);
  end 
  else
  begin
    Dec(x);
  end;

Man hat also das begin end aus dem while Sourceblock entfernt, aber ALLES nach dem while ist weiterhin 2 Zeichen grafisch eingerückt. Jeder PASCAL Bezeichner ist steht dabei am Anfang dieser Einrückung, nicht irgendwo.

Nun entfernen wie die unnötigen begin end blöcke

Delphi-Quellcode:

while x > 0 do
  if x < 5 then
    Inc(x);
  else
    Dec(x);

Das sieht schonmal sehr gut aus. Alles ist noch in 2'er Einrückungen Anweisung für Anweisung an ihrer grafischen Position links stehend.

Abfragen wie IF THEN ELSE wurden weiter vereinfacht indem die Regel hinzukam das man deren Anweisungen auf gleiche Zeile hochziehen kann. Die betrifft auch das else. Dabei ist aber zu beachten das man alles hochzieht.

Delphi-Quellcode:

while x > 0 do
  if x < 5 then Inc(x)
    else Dec(x);

Der else Zweig sollte nun ebenfalls 2 Zeichen eingerück werden da er defakto ja ein eigener Sourceblock darstellt und ohne die Einrückung der if then Sourceblock damit grafisch verschmelzen würde.

Wichtig ist einfach:
- man liest von Link nach Rechts
- eine 2 Zeichen Einrückung markiert grafisch einen Sourceblock, sprich einen lokalen Gültigkeitsbereich, oder eine Programmverzweigungsblock.
- am Anfang dieser Einrückungen steht der PASCAL Sprachbezeichner und danach das was passiern soll, also immer LINKS->RECHTS.

Delphi-Quellcode:

// Links      Rechts

while         -> x > 0
  if          -> x < 5  -> inc(x)
    else                -> dec(x)

Man sieht sehr deutlich die sich grafisch ergebende Hierarchie der Abarbeitung im Programmfluß.

Gruß Hagen

[negaH]

Das Auseinanderreißen von Sparchkostrukten ist ebenfalls schlechter Stil, also sowas:

Delphi-Quellcode:

if x > 0
  then

while x > 0 
  do

repeat
until 
  x > 0;

Bei all diesen Konstrukten gehört alles in eine Zeile, also so

Delphi-Quellcode:

while Bedingung do

if Bedingung then

repeat
until Bedingung;

Das ist ganz einfach begründet, denn die EINFACHSTE Programmverzweigung, nämlich keine, wird durch ein BEGIN markiert.

Delphi-Quellcode:

function XYZ(): Result;
begin                     // <-
   
  begin                   // <- separater Programblock
    ABC;
  end;

end;

Die bedingten Programmverzweigungen ersetzen nun einfach dieses begin

Delphi-Quellcode:

function XYZ(): Result;
begin                     // <-

// be-         -gin
  if Bedingung then                 // <- separater Programblock
    ABC;

  
// be-         -gin
  while Bedinung do
    ABC


  repeat
    ABC;
  until Bedingung;       // logisch betrachtet einfach mal umdrehen und so betrachten

  until Bedingung   
    ABC;
  repeat
     
end;

Gruß Hagen

[sECuRE]

Hi Hagen,

Zitat von negaH:

@Secure:

sorry aber [...] halte ich fr ziemlich übel.

1.) Du vereinfachst durch eigene Annahmen die gewünschte Funktionsweise des Programmieres, indem du enifach mal davon ausgehst das man eh nur Zahlen bis 2^31 testen möchte, statt eben wie im Original mit Int64 zu rechnen. Das ist wohl sehr schlechter Stil. Die Aufgabe des programmieres ist es die Forderungen EXAKT umzusetzen.

Was war denn die Forderung? Ich habe nur gelesen, dass es sich um ein Programm zum Primfaktorzerlegung handelt und gesehen, dass der Autor die Zahlen über ein Edit via StrToInt einliest. Folgedem ist ein integer ideal, da er - im Gegensatz zu einem int64 - schneller berechnet wird und alle Werte, die StrToInt zurückliefern kann, aufnehmen kann. Die Aufgabe, die ich mir beim Programmieren setze, ist, optimal zu Programmieren, in erster Linie in Hinblick auf Geschwindigkeit, im zweiten Hinblick auf Ästhetik.

Zitat von negaH:

2.) dein Source ist fehlerhaft. Was ist mit 0, oder -1 oder allen Zahlen bis -(2^31-1). Übergibt man deiner Funktion einen Integer im Wertebereich der Integer von -2^31 bis 2^31 dann macht sie defacto exakt 50% Fehlerhafte Ausagen.

OK, habe ich verbessert.

Zitat von negaH:

3.) dein Schreibstil, hm naja. Das Besondere in PASCAL ist es das man Gorß/Klein Schreibung als Mittel zu besseren Lesbarkeit einsetzen kann. Du verzichtest darauf indem du alles klein schreibst. Damit verzichtest du auf Übersichtlickeit.

Großschreiben wird allgemein als Schreien angesehen, und ich schreie meinen Compiler/Computer/wasauchimmer nicht an . Außerdem verwende ich sehr wohl Groß-/Kleinschreibung, allerdings hauptsächlich bei Variablen- und Funktionsnamen, dort, wo es eben sinnvoll ist.

Zitat von negaH:

4.) N. Wirth hat mit Absicht die "Gültigkeit eines Source Blocks" visuell grafisch auch als eingerückter Block mit einem senkrecht untereinander stehendem BEGIN und END eingeführt. Es ist also essentiell das BEGIN und END auf gleicher Einrückungsebene zu schreiben.

Ich finde, dass sich der Quelltext dadurch nur um eine weitere Zeile aufbläht. Logisch ist die genaue Einteilung mit begin/end ohnehin für Menschen nicht (IMO): Schließlich ist es ja egal, wie viele Anweisungen danach folgen (nach einer if-Bedingung zB), es muss nur dem Compiler genau gesagt werden.

Zitat von negaH:

5.) was nun ? IstPrim() oder IsPrime() ? aber nicht IsPrim() das ist denglish.

Wenn du genau hinschaust, wirst du feststellen, dass ich IstPrim geschrieben habe.

Zitat von negaH:

6.) in der Mathemtik ist es üblich N als Synonym für eine Natürlich Zahl zu betrachten, so wie R für rationelle Zahlen etc.pp.

Ich bin Programmierer und kein Mathematiker.

Zitat von negaH:

7.) Sonderfall 1 ist zu wenig und berücksicht eben nicht negative Zahlen

Hast du mir doch in 2.) schon gesagt?

Zitat von negaH:

8.) die Berechnung in for X := 0 to Trunc(Sqrt(n)) ist zwar auf Grund des Delphi Compilers möglich sollte aber aus Sicht eines Standard PASCAL Codes vermieden werden. Unter Umständen, bei erweiterte PASCAL Dialekten mit einer for i to x step y Syntax ist nämlich x als Schleifenendebedingung dynamisch veränderbar.

Ich programmiere Delphi, kein Pascal.

Zitat von negaH:

9.) wenn es geht sollte man immer auf unnötige Sprachkonstruke verzichten und statt dessen mit "mathematischen" Zuweisungen arbeiten.

Hast du dafür ein konkretes Beispiel?

Zitat von negaH:

10.) wenn es geht exit/goto/continue vermeinden

Warum? Es ist doch die effizienteste Methode, hier aus der Schleife zu springen, oder sehe ich das falsch?

Zitat von negaH:

11.) man muß nur mit ungeraden Zahlen die Trialdivision durchführen. Da es aber in Delphi keine for i tox step y Schleife gibt müsste man sich behelfen mit for x := 2 to Trunc(Sqrt(N)) div 2 do und mit if N mod (i*2+1) = 0 arrbeiten, um eben den offensichtlichen mathematischen Erfordernissen gerecht zu werden. Es geht hier um Exaktheit in der Programmier Arbeit die man leistet. Allerdings ist eine solche Zähleschleife wesentlich schlechter zu verstehen als eine while Schleife. In deinem Falle testet deine Funktion die Zahl N doppelt so häufig als es nötig wäre, ergo Effizienzeinbußen.

Hmm, da kann' ich so spontan nicht zustimmen. Was ist zum Beispiel mit 8? Die Wurzel von 8 ist 2, das dann nochmal geteilt durch 2 ist 1 und deine Schleife würde gar nicht ausgeführt!

Ich hab' meinen Source verbessert (war übrigens auf Grund von zwei Tippfehlern vorher nicht lauffähig):

Delphi-Quellcode:

function IstPrim(const n:integer):boolean;
var x:integer;
begin
  // Sonderfälle beachten
  if (n <= 1) then begin
    result:=false;
    exit;
  end;

  result:=true;
  for x:=2 to Trunc(Sqrt(N)) do
    if ((n mod x) = 0) then begin
      result:=false;
      exit;
    end;
end;

// eine meiner Lieblingsfunktionen ;)
// bildet das (<condition>?<true>:<false>)-Konstrukt aus C/PHP nach
function iif(const Condition:boolean; const IfTrue,IfFalse:string):string;
begin
  if (Condition) then
    result:=IfTrue
  else result:=IfFalse;
end;

// integer: siehe oben
procedure Zerlegen(var OutputLabel:TLabel; const n:integer);
var x:integer;
begin
  x:=2;
  while ((n mod x) <> 0) do
    Inc(x);

  // Korrektur: OK, so schwachsinnig war sie doch nicht. Hab' mich verlesen.
  OutputLabel.Caption:=OutputLabel.Caption+iif(length(OutputLabel.Caption) > 0,'*','')+IntToStr(x);

  if (not IstPrim(n div x)) then
    Zerlegen(OutputLabel,n div x);

  m:=m*x;
end;


procedure TForm1.Button1Click(Sender: TObject);
begin
  m:=1;
  label2.Caption:='';
  if (IstPrim(strtoint(edit1.Text))) then
    Label2.Caption:=edit1.Text+' ist eine Primzahl!'
  else begin
    Zerlegen(Label2, StrToInt(Edit1.Text));
    Label2.Caption:=Label2.Caption+'*'+IntToStr(StrToInt(Edit1.Text) div m);
  end;
end;

cu

[himitsu]

Zuviel rede ich eh nicht mit, da ich ja sowieso meinen eigenen "guten" Programierstil hab

Aber zu dem ELSE, meiner Meinung nach hat das die selbe Wertigkeit wie das IF und wird demnach nicht eingerückt.

Delphi-Quellcode:

If ... Then ...
Else ...;

// bei kurzen Blöcken auch mal 
If ... Then ... Else ...;

If ... Then Begin
  ...;
End Else ...;

If ... Then Begin
  ...;
End Else Begin
  ...;
End;

If ... Then Begin
  ...;
End Else If ... Then Begin
  ...;
End Else Begin
  ...;
End;

Das BEGIN ziehe ich mit auf die Zeile mit der Bedingung/Schleife ... daß die nachfolgenden Codezeilen zu der Bedingung/Schleife gehöen sehe ich ja an der Einrückung ^^

Delphi-Quellcode:

If ... Then Begin
  ...;
  ...;
  ...;
End;

If ... Then
  ...;
...;
...;

If ... Then ...;
...;
...;

Und irgendwie finde ich meine Version übersichtlicher, als eure ._.

Delphi-Quellcode:

// MEINE
If ... Then Begin
  ...;
End Else If ... Then Begin
  ...;
End Else Begin
  ...;
End;

// das Andere
If ... Then
Begin
  ...;
End
Else
  If ... Then
  Begin
    ...;
  End
  Else
    Begin
      ...;
    End;

Und ja, ich mache manchmal immernoch mehrere Befehle in eine Zeile, wenn es logisch zusammengehört und so auch noch übersichtlicher ist



[add]
zu der einen Funktion ... wie siht das für dich aus?

Delphi-Quellcode:

function IstPrim(const n: integer): boolean; 
var i: integer; 
begin 
  result := false; 
  if n <= 1 then exit;          // Sonderfälle beachten 
  for i := 2 to Trunc(Sqrt(N)) do 
    if n mod i = 0 then exit; 
  result := true; 
end;

[turboPASCAL]

Delphi-Quellcode:

// A) IF - Kurtzschreibweise
if ... then  ...;

// B) IF - Kurtzschreibweise mit Elsezweig
if ... then ... else ...;

// C) IF - Kurtzschreibweise mit Elsezweig in neuer Zeile
if .......... then ........
  else .......... ;

// D) IF - Schreibweise mit Elsezweig 
if ....................
  then ....................;
  else ....................

// D) IF - BlockSchreibweise
if .................... then
begin
  ....................;
  ....................
end;

// E) IF - BlockSchreibweise und Else-Zweig
if .................... then
begin
  ....................;
  ....................
end else
begin
  ....................;
  ....................
end;

// oder:
if .................... then
begin
  ....................;
  ....................
end
else
begin
  ....................;
  ....................
end;

// auch ne Art, kann aber in der VCL lang wedern ;-)
if ...... then ...... else
  if ...... then ...... else
    if ...... then ...... else .....;

// kann ich nich leiden ;-)

// 1.
if .................... then begin
  ....................;
  ....................
end
else begin
  ....................;
  ....................
end;


// 2. ( unübersichtlich )
if ... then ...
else ...

[tigerman33]

Also, ich denke solange sich der Programmierer bei seinen Einrückungen was gedacht hat und vor allem den Stil konsistent durchzieht (!!) macht es nicht so den großen Unterschied ob er jetzt

Delphi-Quellcode:

if ... then begin
end else begin
end;

oder

Delphi-Quellcode:

if ... then 
begin
end 
else 
begin
end;

oder noch irgendwas anderes schreibt. Ist auch zu einem gut Teil Geschmackssache. Nur durcheinander würfeln sollte man das nicht.