Hi zusammen

Ich habe hier ein Demoprogramm mit einem für mich doch recht exotischen Konstrukt:

zusammenfalten · markieren

Delphi-Quellcode:

// SelectedIndex := XXX.CharacterIndexAtPos(X, Y, Tolerance, Tolerance);
// if (not Selecting) and (XXX.WebLinkAtPos(X, Y) <> -1) then
// XXX.Cursor := crHandPoint
// else if SelectedIndex >= 0 then
// XXX.Cursor := crIBeam
// else
// XXX.Cursor := crDefault;
{*  if Selecting then
    if SelectedIndex >= 0 then
    begin
      NeedRepaint := False;
      if SelectionStart = -1 then
      begin
        SelectionStart := SelectedIndex;
        NeedRepaint := True;
      end;
      if SelectionEnd <> SelectedIndex then
      begin
        SelectionEnd := SelectedIndex;
        NeedRepaint := True;
      end;
      if NeedRepaint then
        XXX.Invalidate;
    end; *}

Im ersten Teil ist das if..then..else-Konstrukt für mich eher gewöhnungsbedürftig, dürfte so aber durchaus möglich sein.
Etwas anders sieht es im zweiten, mit Klammern/Sternchen auskommentierten Teil aus. Die erste Zeile dieses Blocks beginnt mit einem einfachen if..then.
Daran schliessen sich mehrere if..then..begin..end einfache Blöcke an.

Wenn ich das so lese, müsste, wenn die 1. Zeile nicht zutrifft, die Prozedur beendet werden (beim nächsten, hier nicht gezeigten end). Aber was mich am meisten irritiert ist, dass nach der ersten if-Abfrage mehr als eine Zeile folgt (Folgen kann), die nur in Abhängigkeit dieser Abfrage(true) ausgeführt wird.

Wie seht ihr das?

Gruss
Delbor

"Mehr als eine Zeile" ist nicht die exakte Regel die gilt wenn kein begin/end genutzt wird. Genauer ist eine Anweisung ODER ein Block.
In dem Fall steht der gesamte "if SelectedIndex >= 0 then"-Block unter dem ersten if.

Es ist sehr schlecht und unleserlich eingerückt und man sollte gerade wenn es nicht um wirklich nur 1 Anweisung sondern einen ganzen Block geht begin/end nutzen:

Schlecht (so ist es jetzt):

markieren

Delphi-Quellcode:

if Selecting then
if SelectedIndex >= 0 then
begin
  ...
end;

Besser:

markieren

Delphi-Quellcode:

if Selecting then
  if SelectedIndex >= 0 then
  begin
    ...
  end;

Gut:

markieren

Delphi-Quellcode:

if Selecting then
begin
  if SelectedIndex >= 0 then
  begin
    ...
  end;
end;

Alle 3 Versionen funktionieren am Ende gleich.

Die Einrückung irritiert.
Zudem würde ich statt

markieren

Code:

if Selecting then
if SelectedIndex >= 0 then begin ... end

eher schreiben

markieren

Code:

if Selecting and (SelectedIndex >= 0) then begin ... end

denn das macht es viel klarer.

Das Einrücken durch die gschweifte Klammer ist mist. Die Klammer muss auf eine extra Zeile. Generell ist das alles auskommentiert zum aktuellen Zeitpunkt. Was soll nun ausgeführt werden? Nur die // oder alles zwischen {**}?
Aufgeräumt sieht das bei mir so aus:

zusammenfalten · markieren

Delphi-Quellcode:

  SelectedIndex := XXX.CharacterIndexAtPos(X, Y, Tolerance, Tolerance);
  if (not Selecting) and (XXX.WebLinkAtPos(X, Y) <> -1) then
    XXX.Cursor := crHandPoint
  else if SelectedIndex >= 0 then
    XXX.Cursor := crIBeam
  else
    XXX.Cursor := crDefault;

  if Selecting then
  begin
    if SelectedIndex >= 0 then
    begin
      NeedRepaint := False;
      if SelectionStart = -1 then
      begin
        SelectionStart := SelectedIndex;
        NeedRepaint := True;
      end;
      if SelectionEnd <> SelectedIndex then
      begin
        SelectionEnd := SelectedIndex;
        NeedRepaint := True;
      end;
      if NeedRepaint then
        XXX.Invalidate;
    end;
  end;

Also sind es eigentlich zwei Blöcke.

Hi zusammen

Danke für eure Antworten. Persönlich halte ich es auch so, dass ich auch da ein begin..end setze, wo es eigentlich nicht nötig wäre, ausser so ein Einzeiler steht gut erkennbar alleine.

Ich hab gerade jetzt wieder so einen Fall, dass Delphi meint:

Zitat:

')' erwartet, aber '.' gefunden in Klasse TViewerFormMain.

Dabei hatte ich erst einen Prozedurkopf angelegt mit begin..end und wollte die Deklaration mit Shift/Ctrl+C anlegen. Wo der Punkt ist, ist keine Frage, aber die Klammer könnte in wirklichkeit auch ein fehlendes end sein - und das im kompletten Code über dem Punkt. Da wünsche ich mir schon mal viel Spass...

Ach ja: Ich habe den oben auskommentierten Code aus dem bisherigen Eventhandler ins 'Zwischenarchiv' verschoben und durch eigenen Code (zT. von hier) ersetzt. Und genau da muss ich mir die Sache 'eingebaut' haben.

@Sinspin : Ich habe den Code auskommentiert, weil ich ihn zumindest vorerst nicht mehr brauche - da ich ihn noch nicht wirklich verstehe, hab ich ihn auch noch nicht gelöscht. Zwei verschiedene Auskommentierungen habe ich verwendet, um die einzelnen Codezeilen/Abschnitte zu verdeutichen


Gruss
Delbor

Zitat von Frickler:

Die Einrückung irritiert.
Zudem würde ich statt

markieren

Code:

if Selecting then
if SelectedIndex >= 0 then begin ... end

eher schreiben

markieren

Code:

if Selecting and (SelectedIndex >= 0) then begin ... end

denn das macht es viel klarer.

Ich sehe es genau anders herum. Die zwei If-Anweisungen kann man viel besser debuggen.

Einfach durchsteppen ohne den Inhalt der Variablen ansehen zu müssen.

Zitat von Frickler:

markieren

Code:

if Selecting and (SelectedIndex >= 0) then begin ... end

Das geht für mich so garnciht, weil cih so keien Breakpint auf den Teil nach then setzen kann. Daher mache ich nach jedem then eine neue Zeile. Ob dann eine begin/end block kommt hängt davon ab, ob dieser aus mehr als einer Statementzeile besteht.

zusammenfalten · markieren

Delphi-Quellcode:

if a = b then
  c := a;

if a = b then
begin
  c := a;
  d := b;
end;

if a = b then
begin
  if a = b then
  begin
    c := a;
    d := b;
  end;
end:

Zitat von Delbor:

Wo der Punkt ist, ist keine Frage, aber die Klammer könnte in wirklichkeit auch ein fehlendes end sein - und das im kompletten Code über dem Punkt. Da wünsche ich mir schon mal viel Spass...

Automatische Codeformatierung ausführen, dann siehst du sofort, wo das Problem liegt...

Ob ein oder zwei IF ... meistens Geschmackssache.
Aber wenn man auf die saudämiche Idee kommen sollte die "vollständige boolische Auswertung" zu aktivieren, dann sind zwei IFs eh oft zwingend notwendig.

Ich tendiere eher zum AND
und Andere machen das dann wieder "kaputt", indem sie jeden AND/OR/+/...-Teil auf eine neue Zeile schieben.
Aber ist auch OK, allerdings nehme ich dann die Operatoren nach vorhne, damit man direkt erkennt, dass es da weitergeht,
während Andere die Operatoren gern hinten "verstecken".

unvmm.



Das mit dem Haltepunkt stimmt auch nicht ganz, denn du kannst den Haltepunkt auch im Assembler auf den Teil hinterm AND setzen. (könnte man über einen netten Debughelper bestimmt auch aus der PAS erledigen)

Außerdem reicht auch ein Haltepunkt zu Beginn und dann sieht man sich eben die Werte im Debugger an (dafür ist er ja da), oder gibt es als "Bedingung" für den Haltepunkt an.