Beurteilung meines Quelltextes

[Stefan92]

Ok, danke für die schnellen Antworten und für den Tip mit dem Speedbutton, klappt auch (war ja auch nicht anders zu erwarten :zwinker: ). Aber eine Frage:
Wie findet ihr meinen Quelltext? Gibt es da überflüssige Stellen oder unsaubere Passagen?

Gruß

Stefan

PS:Wenn ihr nicht wollt, müsst ihr euch den Code nicht durchlesen, ist ja auch ziemlich lang, das ist rein optional!

[edit=Christian Seehase]Dies ist ein Thread der aus diesem entstanden ist. Der angesprochene Quelltext ist dort zu finden, und wurde von mir hier als Anhang hinzugefügt Mfg, Christian Seehase[/edit]

[Nils_13]

Den Code habe ich nur überflogen und kann darüber schlecht urteilen, aber:
1. Bennene mal deine Komponenten um, in einem halben Jahr kannst du damit nichts mehr anfangen. Da musst du selbst ein System finden, z.B. bAddition (b=Button), je nachdem, was dir zusagt. Gewöhn es dir gleich an und du programmierst besser, als ich es in deinem Alter getan habe.
2. Die Codeformatierung ist nicht gerade übersichtlich. Wenn du mal >2500 Zeilen-Projekte programmierst, wirst du es noch verstehen, weil es einfach (gerade bei mehreren If-Abfragen!) total unübersichtlich ist und du auch langsamer sein würdest. Du hast aber im Vergleich zu vielen Anderen noch eine anschauliche Codeformatierung, die nicht so schlimm ist, nur um dir einen besseren Weg zu zeigen. Da kann ich nur dieses Tutorial empfehlen: http://www.dsdt.info/grundlagen/styl...infuehrung.php

[jakobwenzel]

Wenn du ein richtiges Projekt machen willst, dann benenn die Controls um. Weißt du nach einem Monat noch (ohne Kommentare im Code) noch, was Button237 macht? Oder wüsstest du dann noch, was der Button btnSaveSettings macht? :wink:

//EDIT: Zu lahm, aber mit Beispiel, deshalb send ichs mal trotzdem

[Matze]

Styleguide bleibt jedem selbst überlassen, wobei deiner in den meisten Teilen recht gut lesbar ist, da gibt's deutlich schlimmere. Dazu gäbe es aber in der DP einige Diskussionen, also falls du wissen möchtest, wie man etwas formatieren kann (Einrückungen etc.) einfach suchen. ;)

An deiner Stelle würde ich auf jedenfall den Komponenten aussagekräftigere Namen geben. "Button1" lässt nicht darauf schließen, was dieser Button macht. Wobei es bei dir hier zufälligerweise ok ist, da er auf die Taste 1 reagiert. Aber beispielsweise "Button15", da wäre "BtnPlus" oder ähnliches, deutlich schöner.


PS: Da ich ein paar Beispiele drin habe, sende ich es dennoch ab, auch wenn's z.T. bereits genannt wurde.
PPS: Aber eigentlich gehört sowas nicht in dieses Thema ...

[Nils_13]

Zitat:

Zitat von Matze

Styleguide bleibt jedem selbst überlassen, wobei deiner in den meisten teilen gut lesbar ist. Dazu gäbe es aber in der DP einige Diskussionen, also falls du wissen möchtest, wie man etwas formatieren kann (Einrückungen etc.) einfach suchen. ;)

Jp, aber bei langen Prozeduren mit vielen Variablen, bin ich sogar wieder zum Anfang zurückgekehrt (naja nicht ganz) und schreibe gruppiert
s1 : String; // Kommentar
s2 : String // Kommentar
und rücke es ein. Dann findet man sich nach zig Jahren noch recht schnell in den Code rein, weil man ihn dadurch auch sehr gut kommentieren kann. Mir ist auch aufgefallen, dass mir nach lästigen if-else-Klauseln immer die Augen etwas weh taten....
Das

Delphi-Quellcode:

if ... then begin 
  ... 
end
else
begin
  ...
end;

wurde zu:

Delphi-Quellcode:

if ... then
begin
  ... // Falls uneingerückt, ist dies nicht meine Schuld --> Highlighter
end else
begin

end;

Darüber kann man Stunden diskutieren, also lassen wir das. Du hast schon recht, dass jeder seinen eigene Formatierung haben sollte, aber viele kommen doch erst später zu einer einheitlichen Codeformatierung.

*** EDIT ***
Verdammt, habe dein zweites PS übersehen, sry.

[Sunlight7]

'n Abend!

Ich hab Deinen Code auch nur kurz überflogen :-D

Was mir auffiel:

Statt

Delphi-Quellcode:

or (operation=true)

genügt

Delphi-Quellcode:

or operation

Bei if auch:

Delphi-Quellcode:

If strich=true then

=

Delphi-Quellcode:

If strich then

Gibt in der DP auch Threads zu dem Thema.

Der Typ Real ist Out :zwinker:

Delphi-Quellcode:

var vorzeichenwechsel:real; = var vorzeichenwechsel:Single;

Grüßle!

PS: Wenn Du Code Postest, dann bitte mit den richtigen Tags, in diesem Fall Delphi-Tags, damit man das besser lesen kann :zwinker:

[shmia]

Schau mal folgenden Code an, der wiedeholt sich mindestens 10 Mal mit ganz geringen Unterschieden:

Delphi-Quellcode:

procedure TForm1.Button10Click(Sender: TObject); 
begin 
  if (panel1.caption='0') or (operation=true) then 
  begin 
    panel1.caption:='0'; 
    operation:=false; 
  end 
  else panel1.caption:=panel1.caption+'0'; 
  if strich=true then zahl:=true; 
  panel1.setfocus; 
end;

Wenn man ein Codefragment so oft sieht, muss sofort die Alarmglocke UNTERFUNKTION klingeln!!
Hier die Unterfunktion mit dem Parameter "zahl":

Delphi-Quellcode:

procedure TForm1.ZahlKlick(zahl:char); 
begin 
  if (panel1.caption='0') or (operation=true) then 
  begin 
    panel1.caption:=zahl; 
    operation:=false; 
  end 
  else panel1.caption:=panel1.caption+zahl; 
  if strich=true then zahl:=true; 
  panel1.setfocus;end;

Damit ergibt sich folgende Vereinfachung (und zwar für 10 Buttons):

Delphi-Quellcode:

procedure TForm1.Button10Click(Sender: TObject); 
begin 
   ZahlKlick('0');
end;

Deine Art zu programmieren nennt man "Programmierung mittels Copy & Paste" und erzeugt sehr viel Programmcode, der nur schwer zu debuggen ist.
http://de.wikipedia.org/wiki/Antipat...-Anti-Patterns

[Stefan92]

@Sunlight7:Danke für den Typ mit dem Boolean-Variablen, aber was bedeutet, dass Real out ist? Ist es schlechter als Single oder Double?
@all: Danke schön für die ganzen Tips, ich wusste nicht, dass ich so viel falsch mache... :-D
Ich weiss, ich bin ein bisschen offtopic geworden, kann man den Titel denn noch nachträglich ändern?

Gruß

Stefan

[Christian Seehase]

Moin Stefan,

ich schlage mal vor Du überlegst Dir einen Titel, schickst mir den, und ich teile das Thema, so dass ein neuer Thread entsteht.

[Stefan92]

Hallo Christian, (für Moin ist es wohl etwas zu spät... :))
als zweiten Titel würde ich 'Wie findet ihr mein Quelltext' oder falls zu lang 'Beurteilung meines Quelltextes' nehmen.
Danke fürs Angebot.

Gruß

Stefan

Edit: Ups, sehe grade, dass der zweite Titel nur ein Zeichen länger ist... :gruebel: Nimm halt irgendeinen.

[Sunlight7]

'n Abend Stefan:

In der Delphi Hilfe steht geschrieben:

Der Typ Real (Real48) wird wegen der Abwärtskompatibilität mit früheren Versionen von Delphi und Borland Pascal bereitgestellt. Da das Speicherformat des Typen Real in den Intel-CPUs nicht vorgesehen ist, sind Operationen mit Real-Typen langsamer als die anderen Fließkommatypen.

Grüßle!

[Stefan92]

Morgen Sunlight7,
OK, ab sofort benutze ich Double.

Gruß

Stefan

[uwewo]

Hallo Stefan,

habe Deinen Source auch nur überflogen, aber hier eine kleine ungetestete Idee die den Code um ein paar Zeilen kürzt. Lösche alle Proceduren von Button2Click bis Button10Click, und ersetze Button1Click mit diesem.

Delphi-Quellcode:

procedure TForm1.Button1Click(Sender: TObject);
begin
 with Sender as TButton do
   begin
    if (Panel1.caption = '0') or (operation = true) then
      begin
        Panel1.caption := (Sender as TButton).Caption;//Beschriftung des Buttons der angeklickt wurde
        operation := False;
      end
      else
       Panel1.caption := Panel1.caption + (Sender as TButton).Caption
    if strich then zahl := True;
    Panel1.setfocus;
   end;
end;

Dann kannst Du im OI von Button1Click - Button10Click den Source von Button1Click verwenden.

Wie gesagt ungetestet, bin mir aber zu 99% sicher das es funktioniert.

Uwe

PS: Noch etwas ist mir aufgefallen, Du musst PI nicht als Zahl eingeben, dafür gibt es die function Pi() in Delphi ;-)

[IngoD7]

[OFF-Topic ON]
Ich hätte einen Tipp für die Lesbarkeit deiner Postings:
Mache aus deiner Signatur doch bitte eine Signatur und keinen Roman mit Quote-Tags und dergleichen.
Das derzeitige Ding lenkt fürchterlich ab.
Falls du es einsiehst - Danke. ;-)
[OFF-Topic OFF]

[Union]

Das

Zitat:

Zitat von shmia

Damit ergibt sich folgende Vereinfachung (und zwar für 10 Buttons):

Delphi-Quellcode:

procedure TForm1.Button10Click(Sender: TObject); 
begin 
   ZahlKlick('0'); 
end;

läßt sich noch weiter optimieren, indem Du eine der Routinen in ZahlButtonClick umbenennst und sie im OI direkt an alle entsprechenden Controls zuweist.

Des weiteren solltest Du aus dem Code vielleicht eine echte Klasse machen. Du verwendest viele globale Variablen. Dadruch kan es zu Querschlägern kommen, besonders wenn Du Deinen Code in andere Programme einbinden willst.

Hier noch ein Verschönerungsvorschlag. Anstelle der riesigen case Abfrage in FormKeyPress könntest Du die char-Werte in den Tag der Buttons setzen.

Kommentare: außer {+} und {-} fehlen jegliche Anmerkungen. Weder was das Programm macht, noch die einzelnen Routinen.

[Stefan92]

@uwewo:

Zitat:

Dann kannst Du im OI von Button1Click - Button10Click den Source von Button1Click verwenden.

Wie mache ich das denn mit dem Verwenden eines anderen Sources im OI? Hab sowas noch nie gemacht. Das ist ja überhaupt gerade mal mein allererstes Programm mit Delphi, davor hab ich nur mit Free Pascal programmiert.

@IngoD7: Okay, hab die Signatur geändert.

@Union: Also 1. Wie mach ich das mit dem OI?
2. Da ich (fast) gar keine Ahnung von OOP habe, frage ich mich, wie man eine eigene Klasse macht? Gibt es hier möglicherweise ein Tutorial dazu?
3. Wofür ist die Eigenschaft Tag denn gut?
4. Danke für den Tip mit den Kommentaren, ich werd meinen inneren Schweinehund mal bekämpfen und da Kommentare reinsetzen.

Gruß

Stefan

[Hansa]

Zitat:

Zitat von Stefan92

@uwewo:

Zitat:

Dann kannst Du im OI von Button1Click - Button10Click den Source von Button1Click verwenden.

Wie mache ich das denn mit dem Verwenden eines anderen Sources im OI?..

Im OI bei Ereignissen eines anklicken. Die ComboBox aufklappen und mal gucken, was da steht.

[Khabarakh]

Zitat:

Zitat von Sunlight7

In der Delphi Hilfe steht geschrieben:

Der Typ Real (Real48) wird wegen der Abwärtskompatibilität mit früheren Versionen von Delphi und Borland Pascal bereitgestellt. Da das Speicherformat des Typen Real in den Intel-CPUs nicht vorgesehen ist, sind Operationen mit Real-Typen langsamer als die anderen Fließkommatypen.

Stefans Hilfe wird etwas Anderes behaupten ;) :

Zitat:

Der generische Typ Real ist in der aktuellen Implementation mit dem Typ Double identisch.

Und ein generischer Typ ist wirklich das Letzte, das "out" werden kann :mrgreen: .

[Sunlight7]

Muß ja wieder in jeder Version anders sein :roll:

Aber ich schreib lieber Double selber, vielleicht kommen sie ja mal auf die Idee Real mit Single identisch zu machen :lol:

[Stefan92]

Da ich mittlerweile schon länger auf Antwort warte, nehme ich einfach mal an, dass ihr den Thread vergessen habt.
Folgende Frage sind noch offen:

Zitat:

2. Da ich (fast) gar keine Ahnung von OOP habe, frage ich mich, wie man eine eigene Klasse macht? Gibt es hier möglicherweise ein Tutorial dazu?
3. Wofür ist die Eigenschaft Tag denn gut?

Bitte um baldige Antwort :( .

Gruß

Stefan

[Union]

Für OOP Tutorials bemühe doch einfach mal die Suchfunktion des Forums (Eingabe "OOP and Tutorial").
Tag ist ein integer Wert, den Du frei verwenden kannst. Z.B. Tastencodes oder Pointer auf Objekte.

[Stefan92]

OK, Tutorial hab ich gefunden.
Noch eine Frage zu dem Tag:
Was müsste ich denn machen, um über den Tag die case-Abfrage zu umgehen? Ich bräuchte doch trotzdem noch Abfragen oder nicht?
Sorry wenn ich falsch liege, aber ich bin halt noch ein Newbie.

Gruß

Stefan

[morp2]

Hier mal ein Tipp meinerseits:
Leite dir eine eigene Klasse von Button ab un füg ihr eine Eigenschaft "Zahl" vom Typ String zu. Dieser Eigenschaft gibts du dann je nach Button die Werte '0' bis '9' (Sollte direkt im Designer gehen). Dann leitest du ALLE Buttonklicks auf eine Methode um (Also im Designer, beim Event Click, für jeden Modifizierten Button, diese Methode angeben):

Delphi-Quellcode:

procedure TForm1.MyButtonClick(Sender: TObject);
var
  MyButton: TMyButton;
begin
  MyButton := TMyButton(Sender);
  ZahlKlick(MyButton.Zahl);
end;