Eure besten Quellcode Kommentare...

[rokli]

Wenns ginge, würde ich Dir dafür ein "Sternchen" geben, MaBuSE! :thumb:

[MaBuSE]

Zitat:

Zitat von rokli (Beitrag 1407403)

Wenns ginge, würde ich Dir dafür ein "Sternchen" geben, MaBuSE! :thumb:

Vielen Dank für die Blumen ... :lol:

[Neutral General]

Zitat:

Zitat von MaBuSE (Beitrag 1407405)

Zitat:

Zitat von rokli (Beitrag 1407403)

Wenns ginge, würde ich Dir dafür ein "Sternchen" geben, MaBuSE! :thumb:

Vielen Dank für die Blumen ... :lol:

I see what you did there :mrgreen:

[Der schöne Günther]

Code:

(kein if, keine Prüfung)
allControllersHealthy := True; // Wir sind Optimisten

[haentschman]

...da ist jeder Kommentar unötig. :cry:

In dem aktuellen Projekt gefunden:

Delphi-Quellcode:

if (not Data.FDFix.Active) then
begin
  Data.FDFix.Active := False;
end;

(Namen zum Schutz des Verursachers geändert :-D)

[Codehunter]

Zitat:

Zitat von haentschman (Beitrag 1426904)

...da ist jeder Kommentar unötig. :cry:

In dem aktuellen Projekt gefunden:

Delphi-Quellcode:

if (not Data.FDFix.Active) then
begin
  Data.FDFix.Active := False;
end;

(Namen zum Schutz des Verursachers geändert :-D)

Was denn... Nur so kann man sicher gehen dass es auch wirklich AUS ist und nicht etwa auf STANDBY :lol:

[MaBuSE]

Zitat:

Zitat von haentschman (Beitrag 1426904)

...da ist jeder Kommentar unötig. :cry:

In dem aktuellen Projekt gefunden:

Delphi-Quellcode:

if (not Data.FDFix.Active) then
begin
  Data.FDFix.Active := False;
end;

(Namen zum Schutz des Verursachers geändert :-D)

Das sollte man aber Kommentieren!!! :shock:
Schau die das Beispiel mal an. Ok ist etwas konstruiert, aber so was in der Art habe ich leider schon gesehen. :(
Nur wenn Active False ist und nochmal False gesetzt wird, passiert was. Du entfernst bei einem Refactoring diese Zeilen und in der Produktion beim Kunden knallts.
Aus diesem Grund sollte so was kommentiert werden !!!.

:duck:

Delphi-Quellcode:

program Data;

{$APPTYPE CONSOLE}
{$R *.res}

uses
  System.SysUtils;

type
  TFDFix = class
  private
    FActive: Boolean;
    procedure SetActive(const Value: Boolean);
  published
    property Active: Boolean read FActive write SetActive;
  end;

var
  FDFix: TFDFix;
  { FDFix }

procedure TFDFix.SetActive(const Value: Boolean);
begin
  if Value = FActive then
    WriteLn('Erwarte das Unerwartete ;)') // In dieser Zeile könnte z.B. etwas wichtiges gemacht werden, was einen Fehler verhindert.
  else
    FActive := Value;
end;

begin
  try
    WriteLn('Start');
    FDFix := TFDFix.Create;
    try
      WriteLn('1: Data.FDFix.Active := True;');
      Data.FDFix.Active := True; // hier passiert nix
      if (not Data.FDFix.Active) then
      begin
        WriteLn('2: Data.FDFix.Active := False;');
        Data.FDFix.Active := False; // das wird ja nie aufgerufen
      end;
      WriteLn('3: Data.FDFix.Active := False;');
      Data.FDFix.Active := False; // hier passiert nix
      if (not Data.FDFix.Active) then
      begin
        WriteLn('4: Data.FDFix.Active := False;');
        Data.FDFix.Active := False; // ups
      end;
    finally
      WriteLn('Ende');
      FDFix.Free;
    end;
    ReadLn;
  except
    on E: Exception do
      WriteLn(E.ClassName, ': ', E.Message);
  end;

end.

Ach ja das Programm bringt folgende Ausgabe:

Code:

Start
1: Data.FDFix.Active := True;
3: Data.FDFix.Active := False;
4: Data.FDFix.Active := False;
Erwarte das Unerwartete ;)
Ende

[Uwe Raabe]

Delphi-Quellcode:

if (not Data.FDFix.Active) then
begin
  Data.FDFix.Active := False;
end;

Wer weiß denn schon, ob

Delphi-Quellcode:

not True

auch wirklich gleich

Delphi-Quellcode:

False

ist? Der Code ist vermutlich schon für den Einsatz auf einem Quanten-Computer vorbereitet. (Schönen Gruß von Erwin Schrödinger)

[haentschman]

Zitat:

Das sollte man aber Kommentieren!!!

...ich meinte nicht das so stehen zu lassen...ich meinte "ohne Worte" :wink:
PS: FDFix war eine Query :stupid:

[MaBuSE]

Zitat:

Zitat von haentschman (Beitrag 1426977)

PS: FDFix war eine Query :stupid:

Dann mach doch einfach folgendes draus, wenn Du dich dann besser fühlst.

Delphi-Quellcode:

...
if Data.FDFix.State in [dsInactive, dsOpening] then Data.FDFix.Close;
...

Das sieht doch gleich viel wichtiger aus. :thumb: :stupid:
Ist aber faktisch das selbe, da:

Delphi-Quellcode:

...
function TDataSet.GetActive: Boolean;
begin
  Result := not (State in [dsInactive, dsOpening]);
end;
...
procedure TDataSet.Close;
begin
  Active := False;
end;
...

ps: Falls es niemand gemerkt hat, dieser Beitrag ist ironisch gemeint. haentschman hat Recht. Das ist natürlich Blödsinn.

[Codehunter]

Da liegt das Problem aber in der Implementierung des Setters:

Zitat:

Zitat von MaBuSE (Beitrag 1426974)

Delphi-Quellcode:

procedure TFDFix.SetActive(const Value: Boolean);
begin
  if Value = FActive then
    WriteLn('Erwarte das Unerwartete ;)') // In dieser Zeile könnte z.B. etwas wichtiges gemacht werden, was einen Fehler verhindert.
  else
    FActive := Value;
end;

begin
  try
    WriteLn('Start');
    FDFix := TFDFix.Create;
    try
      (* ... *)
    finally
      WriteLn('Ende');
      FDFix.Free;
    end;
    ReadLn;
  except
    on E: Exception do
      WriteLn(E.ClassName, ': ', E.Message);
  end;

end.

Das sollte man eigentlich immer so machen, um derlei Probleme zu vermeiden:

Delphi-Quellcode:

procedure TFDFix.SetActive(const Value: Boolean);
begin
  if Value = FActive then 
    Exit 
  else 
  begin
    FActive := Value;
  try
    WriteLn('Start');
    FDFix := TFDFix.Create;
    try
      (* ... *)
    finally
      WriteLn('Ende');
      FDFix.Free;
    end;
    ReadLn;
  except
    on E: Exception do
      WriteLn(E.ClassName, ': ', E.Message);
  end;
  end;
end.

(Hier im Browser editiert, daher bitte die Einrückungen dazu denken)

[MaBuSE]

Zitat:

Zitat von Codehunter (Beitrag 1427010)

Delphi-Quellcode:

procedure TFDFix.SetActive(const Value: Boolean);
begin
  if Value = FActive then 
    Exit 
  else 
  begin
    FActive := Value;
    // ...
  end;
end;

dann bitte aber so:

Delphi-Quellcode:

procedure TFDFix.SetActive(const Value: Boolean);
begin
  if Value <> FActive then 
  begin
    FActive := Value;
    // ...
  end;
end;

Borland/CodeGear/Embarcadero machen das auch so:
Ein schönes Beispiel ist TMargins und TPadding:

Delphi-Quellcode:

 // Quelltext gekürzt !!!

  TMarginSize = 0..MaxInt;

  TMargins = class(TPersistent)
  private
    FLeft, FTop, FRight, FBottom: TMarginSize;
    procedure SetMargin(Index: Integer; Value: TMarginSize);
  protected
    procedure Change; virtual;
  published
    property Left: TMarginSize   index 0 read FLeft   write SetMargin default 3;
    property Top: TMarginSize    index 1 read FTop    write SetMargin default 3;
    property Right: TMarginSize  index 2 read FRight  write SetMargin default 3;
    property Bottom: TMarginSize index 3 read FBottom write SetMargin default 3;
  end;

  TPadding = class(TMargins)
  published
    property Left   default 0;
    property Top    default 0;
    property Right  default 0;
    property Bottom default 0;
  end;

procedure TMargins.SetMargin(Index: Integer; Value: TMarginSize);
begin
  case Index of
    0:if Value <> FLeft then
      begin
        FLeft := Value;
        Change;
      end;
    1:if Value <> FTop then
      begin
        FTop := Value;
        Change;
      end;
    2:if Value <> FRight then
      begin
        FRight := Value;
        Change;
      end;
    3:if Value <> FBottom then
      begin
        FBottom := Value;
        Change;
      end;
  end;
end;

[ConnorMcLeod]

Delphi-Quellcode:

  try
    [... code hier ...]
  except
    // ist mir sowas von egal
  end;

[Sherlock]

Delphi-Quellcode:

  try
[... code zum schreiben in eine Log Datei...]
  except
    // Well, what should we do? What is best practice here?
  end;

¯\_(ツ)_/¯

Sherlock

[Codehunter]

Jetzt nicht direkt Quelltext-Kommentar, aber dennoch einen Platz in dieser Hall of Shame wert: Microsoft

Zitat:

Wenn Sie die Audioverbesserungen deaktivieren, kann dies die Soundqualität verbessern.

Demnach müsste man ja nur ständig Bleifuß fahren, um besonders sparsam unterwegs zu sein. Geile Sache das... :lol:

[TiGü]

Neue Firma, neuer Quelltext - zwar kein Kommentar, aber ich bin gerade drüber gestolpert und wollte es euch nicht vorenthalten.
Da kommt einiges zusammen. :shock:

Delphi-Quellcode:

property ThresholdHyst  :Double read FThresholdHyst write GetTresholdHyst;

[haentschman]

böse...:shock:

[dummzeuch]

Zitat:

Zitat von TiGü (Beitrag 1449424)

Neue Firma, neuer Quelltext - zwar kein Kommentar, aber ich bin gerade drüber gestolpert und wollte es euch nicht vorenthalten.
Da kommt einiges zusammen. :shock:

Delphi-Quellcode:

property ThresholdHyst  :Double read FThresholdHyst write GetTresholdHyst;

Den verstehe ich jetzt nicht: Vermutlich ist das Threshold Hysterese

Was ist daran lustig? (ernstgemeinte Frage)

Ich sehe nur einen Tippfehler: GetTresholdHyst -> GetThresholdHyst

Ok, jetzt hab' ich's auch. Der hat gedauert.

[Codehunter]

Genau, ich rätsele auch noch über den Witz :gruebel:

EDIT: Jetzt hab ichs verstanden :-D

[Der schöne Günther]

write GetTresholdHyst

Wobei die Signatur ja stimmen muss bei den Methoden. Vielleicht wirklich nur ein Tippfehler.


Und ich bleibe dabei, Delphi-Properties haben keinen Zweck, sie sind nur Fehlerquelle und machen das Leben unnötig schwer.

[Codehunter]

Ich würde auf schiefgegangenes Refactoring tippen.

[TiGü]

Jo, Signatur war von dem Setter, aber man stutzt doch schon ungläubig beim ersten Lesen.
Über die historischen Hintergründe kann man nur spekulieren. Ein Tippfehler ist wahrscheinlich.

[Codehunter]

Wir haben hier ja sogar kollektiv drüber weg gelesen obwohl wir direkt drauf gestoßen wurden. Insofern ist es IMHO ein typischer Fall von Flüchtigkeitsfehler ohne technische Relevanz und könnte per Refactoring sicher leicht behoben werden. Seh ich jetzt nicht ganz so kritisch.

[TiGü]

Jup, eine Rakete stürzt davon nicht ab. Finde es halt nur erstaunlich, weil das anscheinend schon seit Anbeginn der Zeit drin ist und noch nie jemanden hier aufgefallen ist.

[Luckie]

Ich habe zu erst den Tippfehler gesehen. Beim Getter fehlt ein H.

[MaBuSE]

Zitat:

Zitat von Codehunter (Beitrag 1449437)

Wir haben hier ja sogar kollektiv drüber weg gelesen obwohl wir direkt drauf gestoßen wurden.

Ich lese immer noch drüber weg. :stupid:
Und wahrscheinlich immer noch im stummen Kollektiv, da sich die anderen nicht outen wollen :hi:

[Codehunter]

Die Rätselauflösung wurde aber schon in #460 gegeben :-)

Wobei mich das Ganze schon an eine Endlosdiskussion mit meinen Kollegen zum Theme Code Style Guides erinnert. Gabs ja hier auch schon. Prinzipiell kann man ja alles benennen wie man will, aber der Wartbarkeit halber sollte man sich an gewisse Regeln halten. Finde mal einen Setter namens Get... zum Bsp. in MMX und ähnlichen Methodenlisten. Da wirst verzweifeln. Oder aber ein Fall von "Code Camouflage Guide - Wie man sich unersetzlich macht" :lol:

[MaBuSE]

Zitat:

Zitat von Codehunter (Beitrag 1449582)

Die Rätselauflösung wurde aber schon in #460 gegeben :-)

Danke, jetzt sehe ich es auch Get <-> Set :shock:

Es ist "Montag", da braucht man manchmal etwas länger. ("*" kann man auch durch jeden anderen beliebigen Tag ersetzen:stupid:)

[Codehunter]

Ist schon faszinierend... TiGü hat da eine richtige Tretmine ausgelegt :-)

|__| <-- Achtung, Fettnapf! Bitte treten Sie nur nach sorgfältiger Planung, ausreichend Vorwarn- und Bedenkzeit sowie angemessen hochwertigem Schuhwerk hier hinein :lol:

[freimatz]

Hätte man den MMX benutzt wäre das wohl nicht passiert.

[Codehunter]

Zitat:

Zitat von freimatz (Beitrag 1449664)

Hätte man den MMX benutzt wäre das wohl nicht passiert.

So sehr ich den auch schätze, er ist nur so gut wie der Anwender ihn nutzt.

[p80286]

Es ist Sonntag und ich schlendere durch die DP.
Das fehlende "h" ist mir durchgegangen, und das "Get.." ist ungewöhnlich, wahrscheinlich wird der Wert aus irgendeinem obskuren Topf geholt? :?:

Ist auf jeden Fall ein Hingucker.

Gruß
K-H

[TiGü]

JEDI JVCL: JvMemoryDataset.pas - Zeile 426 (frisch von Github):

Delphi-Quellcode:

  STATUSNAME = 'C67F70Z90'; (* Magic *)

WTF?

[Stevie]

Zitat:

Zitat von TiGü (Beitrag 1451324)

JEDI JVCL: JvMemoryDataset.pas - Zeile 426 (frisch von Github):

Delphi-Quellcode:

  STATUSNAME = 'C67F70Z90'; (* Magic *)

WTF?

Ok, ich versuch das mal aufzulösen: über git blame findet man den Original Commit.
Dort wird auf Mantis Issue #1447 verwiesen, wo die Codeänderungen ursprünglich gepostet wurden.
Hier wird ein internen Statusfeld benutzt, dem dieser String als Feldname verpasst wird. Woher kommt dieser Wert?

Die Initialen des Authors sind CFZ - die ASCII Codes für C, F und Z sind 67, 70 und 90.

:U+1F92F: (exploding head emoji) :P

[TiGü]

...🤯...

[Codehunter]

Delphi-Quellcode:

initialization

// UnregisterExpectedMemoryLeak  ;

end.

Jetzt kann man sich aussuchen, was man schlimmer findet:

• Dass es überhaupt ein erwartetes Speicherleck gibt
• Dass es Funktionen zum Umgang mit Speicherlecks gibt
• Dass diese Funktionen auch noch in der RTL selbst vorhanden sind
• Dass der Aufruf hier auskommentiert ist
• Dass der auskommentierte Aufruf keinen Parameter hat

[MaBuSE]

Zitat:

Zitat von Codehunter (Beitrag 1452778)

Delphi-Quellcode:

initialization

// UnregisterExpectedMemoryLeak  ;

end.

Jetzt kann man sich aussuchen, was man schlimmer findet:...

"Lustiger" finde ich den Fakt, das im initialization Bereich (, der als erstes ausgeführt wird,) eine Unregister* Funktion angedacht war ;)
Mich würde interessieren wo dann die Register* Funktion aufgerufen wird :stupid:

Ohne mir jetzt die Funktion angesehen zu haben, erwarte ich bei einem Unregister* Aufruf vorher einen Register* Aufruf. Wenn nicht, ist die Benamung der Funktion ... :thumb:

[Codehunter]

Zitat:

Zitat von MaBuSE (Beitrag 1452785)

Ohne mir jetzt die Funktion angesehen zu haben, erwarte ich bei einem Unregister* Aufruf vorher einen Register* Aufruf. Wenn nicht, ist die Benamung der Funktion ... :thumb:

Das ist noch ein Punkt für sich, denn leider sagt die Delphi-Hilfe dazu nur:

Zitat:

Zitat von Delphi-Hilfe

Embarcadero Technologies verfügt zurzeit über keine zusätzlichen Informationen.

Insofern mag ich nicht spekulieren was die nun genau macht. In der Tat gibt es auch ein

Delphi-Quellcode:

RegisterExpectedMemoryLeak

, zu der es sogar eine Hilfe gibt:

Zitat:

Zitat von Delphi-Hilfe

Registriert ein erwartetes Speicherleck.

RegisterExpectedMemoryLeak registriert eine Speicherposition, die eine Anwendung zugewiesen hat und die sie nicht freigeben wird.

Mich würden Szenarien interessieren, in denen Memleaks absichtlich herbei geführt werden (müssen) und dann entsprechend behandelt werden. Wobei ich nach wie vor nicht herausfinden konnte, was (Un)RegisterExpectedMemoryLeak nun eigentlich tut. Womöglich nur das Gemecker im Debugger unterdrücken?

[Der schöne Günther]

Die Memory-Leaks werden sicher nicht absichtlich herbeigeführt, aber wenn du beispielsweise in denen Unit-Tests drin hast dass die Tests fehlschlagen wenn nach Ausführen des Tests mehr Objekte im Speicher sind als vorher dann ist diese Routine dein Freund.

[TiGü]

Oder Lecks von Third-Party-Komponenten, die dir nicht immer unnötig das FastMM4-Log zumüllen sollen.