Hinweis unterdrücken "Auf x zugewiesener Wert wird niemals benutzt"

[Sir Rufo]

Zitat von mkinzler:

In diesem Fall könnte man ausnahmsweise with verwenden:

Delphi-Quellcode:

with TWaitCursor.Create do
  sleep(1000);

Das gibt ein Speicherleck

Zitat von Neutral General:

Zitat von mkinzler:

In diesem Fall könnte man ausnahmsweise with verwenden:

Delphi-Quellcode:

with TWaitCursor.Create do
  sleep(1000);

Muss er ja nicht einmal.

Delphi-Quellcode:

procedure TMainForm.MyMethod;
begin
  TWaitCursor.Create;
  sleep(1000);
end;

Geht auch und produziert keine Warnung. Warum eine Variable anlegen wenn man sie eh nicht verwendet?

Aber ebenfalls ein Speicherleck

[Neutral General]

Stimmt. Dann halt mit Variable und try-finally

[Sir Rufo]

Erweitere die Klasse um eine class function Construct : IWaitCursor;

Delphi-Quellcode:

type
  IWaitCursor = interface
  end;

  TWaitCursor = class (TInterfacedObject, IWaitCursor)
  private
    fCursor : TCursor;
  public
    constructor Create;
    destructor Destroy; override;

    class function Construct : IWaitCursor;
  end;

implementation

class function TWaitCursor.Construct : IWaitCursor;
begin
  Result := TWaitCursor.Create;
end;

constructor TWaitCursor.Create;
begin
  inherited;
  fCursor := Screen.Cursor;
  Screen.Cursor := crHourGlass;
  MainFrom.TrafficLight := tlRed;
end;

destructor TWaitCursor.Destroy;
begin
  Screen.Cursor := fCursor;
  MainFrom.TrafficLight := tlGreen;
  inherited;
end;

dann kann das ohne Speicherleck und Warnung so benutzt werden

Delphi-Quellcode:

procedure Foo;
begin
  TWaitCursor.Construct;
  Sleep( 1000 );
end;

Zitat von Neutral General:

Stimmt. Dann halt mit Variable und try-finally

Wieso?

[Sir Rufo]

Nebenbei bemerkt, ist diese direkte Referenzierung MainFrom.TrafficLight := tlRed; natürlich sehr suboptimal. Damit hast du der Klasse eine sehr starke Abhängigkeit gegeben, die man eigentlich vermeiden soll (eigentlich muss).

Das solltest du anders lösen

[Sir Rufo]

Hier eine Variante ohne diese Abhängigkeiten und Überflüssiges (z.B. IWaitCursor ) und vor allem auch kaskadierbar

Delphi-Quellcode:

unit WaitCursor;

interface

uses
  System.SysUtils,
  Vcl.Forms,
  Vcl.Controls;

type
  TWaitCursor = class( TInterfacedObject )
  private
    class var [weak] _Instance : TWaitCursor;
    class var _OnShow : TProc;
    class var _OnHide : TProc;
  private
    FCursor : TCursor;
  protected
    constructor Create;
  public
    destructor Destroy; override;

    class function Show : IInterface;
    class property OnShow : TProc read _OnShow write _OnShow;
    class property OnHide : TProc read _OnHide write _OnHide;
  end;

implementation

{ TWaitCursor }

constructor TWaitCursor.Create;
begin
  inherited;
  FCursor := Screen.Cursor;
  Screen.Cursor := crHourGlass;
  if Assigned( _OnShow )
  then
    _OnShow( );
end;

destructor TWaitCursor.Destroy;
begin
  _Instance := nil;
  Screen.Cursor := FCursor;
  if Assigned( _OnHide )
  then
    _OnHide( );
  inherited;
end;

class function TWaitCursor.Show : IInterface;
begin
  if not Assigned( _Instance )
  then
    _Instance := TWaitCursor.Create;
  Result := _Instance;
end;

end.

Beispiel

Delphi-Quellcode:

procedure Foo;
begin
  TWaitCursor.Show;
  Sleep( 1000 );
end;

procedure Bar;
begin
  TWaitCursor.Show;
  Foo;
end;

[Der schöne Günther]

Weniger zur Technik, sondern zur Philosophie:

Zitat von Sir Rufo:

Hier eine Variante ohne diese Abhängigkeiten und Überflüssiges (z.B. IWaitCursor ) und vor allem auch kaskadierbar

Kritik soll es nicht sein (ist ja technisch einwandfrei, Daumen hoch auf für das [weak] !), aber ich finde grade bei Dingen wie etwas, das in seinem Konstruktor und Destruktor aktiv das Gesamtbild des Systems verändert es nicht angebracht, das automatisch ablaufen zu lassen wenn es out of scope geht.

Habe ich ein

Delphi-Quellcode:

procedure aufwändigeAufgabe();
begin
   // Schritt 1
   TWaitCursor.Show();
   doStuff();
   doEvenMoreStuff();
   
   // Schritt 2
   transferMoney();
   logStuff();
end;

und jemand spaltet mir das, ohne es besser zu wissen in

Delphi-Quellcode:

procedure aufwändigeAufgabe();
begin
   schritt1();
   schritt2();
end;

procedure schritt1();
begin
   TWaitCursor.Show();
   doStuff();
   doEvenMoreStuff();
end;

[...]

auf, ist das Stundenglas nur noch für Schritt 1 zu sehen. Ja, hätte Refactor-Man es richtig gemacht, hätte er es in aufwändigeAufgabe() gelassen. Aber ich finde den Fehler kann man zu einfach machen.


Ich packe gerne Dinge wie eine lokale TStringList in eine interface-referenzierte Tonne um mir den try..finally-Block für das Aufräumen zu sparen, aber grade im Fall mit dem Cursor wäre mein persönlicher Geschmack wirklich weiterhin ein waschechter try..finally-Block. Da sieht man auch auf den ersten Blick was man hat- Ohne zu wissen, was sich hinter den TWaitCursor -Kulissen abspielt.