Thread - ist das sauberer Code?!

[FAM]

Hallo Zusammen,

habe eine Thread-Routine implementiert und frage mich jetzt ob das sauber geschrieben / ein sauberes Konzept ist.

Ich möchte zyklisch in einen ausgelagerten Thread bestimmte Business-Logik ausführen....

Thread-Lib

Delphi-Quellcode:

unit lib.livedata;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, event.types, DateUtils;

type

  // Ergebniswerte Callback
  TAsyncLivedataResult = record
    ErroText: String;
    ApplicationTimestamp: String;
    OPCConncetionTimestamp: String;
    ErrorCode: Integer; // Fehlercode im Fehlerfall, sonst 0 (=LIVEDATA_OK)
  end;

  TLivedataCallback = procedure(LivedataResult: TAsyncLivedataResult) of Object;

  TLivedataThread = class(TThread)
    FLivedataResult: TAsyncLivedataResult;
    FCallback: TLivedataCallback;

  protected
    procedure Execute; Override;
    procedure DoCallbackVCL;
  public
    constructor Create(Callback: TLivedataCallback); overload;
  end;

function ExecuteAsync(Callback: TLivedataCallback): Boolean;

implementation


  { EventTypes }

constructor TLivedataThread.Create(Callback: TLivedataCallback);
begin

  inherited Create(TRUE); // Thread gestoppt erzeugen
  FreeOnTerminate := TRUE;

  // Callback definieren
  FCallback := Callback;

  // Thread starten, ruft .Execute auf
  Suspended := FALSE;

end;

procedure TLivedataThread.DoCallbackVCL;
begin
  FCallback(FLivedataResult)
end;

procedure TLivedataThread.Execute;
begin

  // Code....
  
 
  // Callback
  if Assigned(FCallback) then // Callback-Funktion übergeben?
    Synchronize(DoCallbackVCL); // dann VCL-fähig ausführen

end;


function ExecuteAsync(Callback: TLivedataCallback): Boolean;
begin
  Result := TRUE;
  TLivedataThread.Create(Callback);
end;

end.

Benutzung der Lib in einer TTimer Komponente

Delphi-Quellcode:

...

uses
  // Livedata-Lib
  lib.Livedata

...

procedure TForm1.LivedataCallback(Result: TAsyncLivedataResult);
begin

  // foobar

end;

procedure TForm1.TimerRedundancyTimer(Sender: TObject);
begin
  // Aufruf Async Thread
  if lib.Livedata.ExecuteAsync(LivedataCallback) then
  begin
    // OK
  end
  else
    // Error

end;

habe das mal durch den Debugger laufen lassen, sieht erstmal OK aus. Im Interval (TTimer) wird der Async ausgeführt, die Frage ist nur ist das sauber oder würdet ihr das anders machen (z.b. Speicherfreigabe,...)?

[baumina]

Ich würde einen Thread niemals über einen Timer aus dem Hauptthread aus aufrufen. Stattdessen würde ich eine while-not-terminated-Schleife im Thread-Executeteil machen. Dort würde ich ein EventWait.WaitFor einbauen, das wie ein Timer wirkt.

[FAM]

Zitat:

Zitat von baumina (Beitrag 1290300)

Ich würde einen Thread niemals über einen Timer aus dem Hauptthread aus aufrufen. Stattdessen würde ich eine while-not-terminated-Schleife im Thread-Executeteil machen. Dort würde ich ein EventWait.WaitFor einbauen, das wie ein Timer wirkt.

Aus welchen Grund würdest du nie einen Timer dafür benutzen?

[baumina]

Warum sollte sich der Hauptthread unnötig mit einem unabhängigen Neben-Thread beschäftigen.

[FAM]

Zitat:

Zitat von baumina (Beitrag 1290300)

Dort würde ich ein EventWait.WaitFor einbauen, das wie ein Timer wirkt.

so?

Delphi-Quellcode:

procedure TLivedataThread.Execute;
var
  i: integer;
begin

  inherited;

  while not Terminated do
  begin

    // Invoke terminate, then call abort funtion
    if Self.Terminated then
    begin

      // call abort-function

      // Exit
      Exit;

    end;


    // Callback-Funktion übergeben?
    if Assigned(FCallback) then
      // dann VCL-fähig ausführen
      Synchronize(DoCallbackVCL);

    Sleep(2000);

  end;

end;

aber damit könnte ich den Thread nie von aussen beenden?!

[baumina]

Delphi-Quellcode:

constructor TLivedataThread.Create(Callback: TLivedataCallback);
begin
  ...
  EventWait := TEvent.Create(nil,true,false,'');
end;

procedure TLivedataThread.Terminate;
begin
  EventWait.SetEvent;

  inherited;
end;

destructor TLivedataThread.Destroy;
begin
  EventWait.Free;

  inherited;
end;

procedure TLivedataThread.Execute;
begin
  while not Terminated do
  begin
  ...
    EventWait.WaitFor(30000);
  end;
end;

Und im Haupthread

Delphi-Quellcode:

  if Assigned(LivedataThread) then
  begin
    LivedataThread.Terminate;
    LivedataThread.WaitFor;
    FreeAndNil(LivedataThread);
  end;

[Sir Rufo]

Schau dir TEvent an.

Im Übrigen läuft so ein Thread erst nach dem Erzeugen los - damit erübrigt sich auch das Erzeugen im Suspended Mode.

[Sir Rufo]

@baumina

Wenn du eine kleine Änderung vornimmst

Delphi-Quellcode:

destructor TLivedataThread.Destroy;
begin
  Terminate;
  EventWait.SetEvent;

  inherited;
  EventWait.Free;
end;

dann reicht ein simples

Delphi-Quellcode:

FreeAndNil( LiveDataThread );

bzw. wenn man die

Delphi-Quellcode:

protected

Methode

Delphi-Quellcode:

TThread.TerminatedSet

überschreibt mit

Delphi-Quellcode:

procedure TLivedateThread.TerminatedSet;
begin
  inherited;
  EventWait.SetEvent;
end;

destructor TLivedataThread.Destroy;
begin

  inherited;
  EventWait.Free;
end;

dann wird es noch weniger Code :)

[baumina]

Danke Rufo, gefällt mir.

[FAM]

Zitat:

Zitat von baumina (Beitrag 1290309)

Und im Haupthread

Delphi-Quellcode:

  if Assigned(LivedataThread) then
  begin
    LivedataThread.Terminate;
    LivedataThread.WaitFor;
    FreeAndNil(LivedataThread);
  end;

hmm, die zeilen verstehe ich nicht? kannst du die bitte einzeln erklären und warum auch in dieser reihenfolge

[Nersgatt]

1. Dem Thread mitteilen, er möge sich bitte beenden
2. Warten, bis der Thread eben dies getan hat
3. Die Instanz freigeben.