Warum den THTTPClient doppelt erstellen?
Im Excecute ist es OK, das im Create war eh ungenutzt.
Assert(FStream <> nil);
Kann niemals NIL sein, denn wenn "doch", dann wirft das Create bereits eine
Exception und es kommt dort nie vorbei.
FOnDownloadComplete erst beim Ausführen zu prüfen ist auch etwas unpraktisch.
Alles war ja sinnlos, wenn diese Methode fehlt, da der Download nur als Stream und von diesem Event verwendet wird.
Ohne das, wurde es vollkommen nutzlos runtergeladen.
Also hier am Besten gleich zu Beginn prüfen und z.B. eine
Exception werfen, bereits im Create.
Delphi-Quellcode:
constructor TFileDownloader.Create(
const URL:
String; OnDownloadComplete: TDownloadCompleteEvent);
begin
if not Assigned(OnDownloadComplete)
then
raise Exception.Create('
peng');
inherited Create(False);
// und jetzt außen kein Start mehr, da es am Ende des Create von selst startet
FreeOnTerminate := True;
FURL :=
URL;
FOnDownloadComplete := OnDownloadComplete;
// und der Rest im Execute/DownloadFile
end;
Für die Funktion zwar nicht relevant, aber
TDownloadCompleteEvent = reference to procedure(Stream: TStream; Success: Boolean);
hat den Vorteil, dass man hier nicht nur Methoden und KlassenMethoden übergegen kann,
sondern auch einfache Prozeduren oder anonyme Methoden.
Den Stream intern erstellen, da wo er auch freigegeben wird, nicht extern.
Nja, da FHttpClient, FSuccess und eigentlich auch FStream im Grunde ausschließlich während der Laufzeit des Threads nötig sind und bezüglich einem threadsaven Zugriffs nicht von außen abgreifbar sein sollten, sind sie als lokale Variable im Execute eh besser aufgehoben.
Das Event als Property, auf welches man auch nach dem Start extern zugreifen kann, fliegt auch raus.
Sowas lässt sich wundeschön als Startparameter übergeben, also als Parameter ans Create und das Start aka Suspended=False ebenfalls.
Die restlichen Variablen/Felder müssen nicht unbedingt global sein.
Durch das FreeOnTerminate sind die Thread-Variable und seine Property von Extern sowieso nicht mehr zu verwenden, da sie per se als ungültig zu betrachten sind, denn von extern weißt du nicht, ob der Thread schon fertig ist und freigegeben wurde.
Selbst wenn der Thread blieb, dann dort als ClassProcedure rein,
aber jetzt einfach mal blöd als einface Prozedur.
Delphi-Quellcode:
TFileDownloader =
class(TThread)
private
FURL:
string;
FStream: TStream;
FOnDownloadComplete: TDownloadCompleteEvent;
procedure DownloadFile;
procedure DoDownloadComplete;
protected
procedure Execute;
override;
public
class procedure Download(
const URL:
String; OnDownloadComplete: TDownloadCompleteEvent);
static;
end;
Und weil jetzt vom der Threadableitung im Grunde nichts mehr übrig ist, lass ich des nachfolgend einfach mal weg.
Delphi-Quellcode:
uses
System.RTLConsts, System.SysUtils, System.Classes, System.Net.HttpClient, System.Net.URLClient;
type
TDownloadCompleteEvent = reference
to procedure(Stream: TStream; Success: Boolean);
procedure ThreadedDownloadFile(
const URL:
String; OnDownloadComplete: TDownloadCompleteEvent);
begin
if (
URL = '
')
or not Assigned(OnDownloadComplete)
then
raise EArgumentException.CreateRes(@SArgumentNil);
TThread.CreateAnonymousThread(
procedure
var
HTTP: THTTPClient;
Stream: TMemorystream;
Success: Boolean;
begin
TThread.NameThreadForDebugging('
ThreadedDownloadFile');
HTTP := THTTPClient.Create;
Stream := TMemoryStream.Create;
try
HTTP.CustomHeaders['
Pragma'] := '
no-cache';
try
HTTP.Get(
URL, Stream);
Success := True;
except
Success := False;
end;
TThread.Synchronize(
nil,
procedure
begin
OnDownloadComplete(Stream, Success);
end);
finally
Stream.Free;
HTTP.Free;
end;
end).Start;
end;
ThreadedDownloadFile('
http://sonst.wo/dat.ei', YourDownloadComplete);
// oder
ThreadedDownloadFile('
http://sonst.wo/dat.ei',
procedure(
const URL:
String; Stream: TStream; Success: Boolean)
begin
if Success
then
Machwas(Stream);
end);
Gut, dass man im OnDownloadComplete zwar mitbekommt, ob es nicht ging, aber nicht warum ... statt eines doofen Success könnte man z.B. die
Exception oder ihre Message übergeben.
Für mehrere Downloads das selbe OnDownloadComplete, dann auch die
URL mit als Parameter dort rein, damit man dort weiß was es ist.
Delphi-Quellcode:
uses
System.RTLConsts, System.SysUtils, System.Classes, System.Net.HttpClient, System.Net.URLClient;
type
TDownloadCompleteEvent = reference
to procedure(
const URL:
String; Stream: TStream; Error:
Exception);
procedure ThreadedDownloadFile(
const URL:
String; OnDownloadComplete: TDownloadCompleteEvent);
begin
if (
URL = '
')
or not Assigned(OnDownloadComplete)
then
raise EArgumentException.CreateRes(@SArgumentNil);
TThread.CreateAnonymousThread(
procedure
var
HTTP: THTTPClient;
Stream: TMemorystream;
Success: Boolean;
DError:
Exception;
begin
TThread.NameThreadForDebugging('
ThreadedDownloadFile');
DError :=
nil;
HTTP := THTTPClient.Create;
Stream := TMemoryStream.Create;
try
HTTP.CustomHeaders['
Pragma'] := '
no-cache';
try
HTTP.Get(
URL, Stream);
{TThread.Synchronize(nil,
procedure
begin
OnDownloadComplete(URL, Stream, nil);
end);}
except
{on E: Exception do begin
TempErr := E; // durch einen Bug muß es kopiert werden, auch wenn der Compiler sich bei OnDownloadComplete(URL, nil, E); nicht beschwert ... E ist im Sync leider NIL
TThread.Synchronize(nil,
procedure
begin
//OnDownloadComplete(URL, nil, E); // siehe Bugreport im alten QualityPotal
OnDownloadComplete(URL, nil, TempErr);
end);
end;}
FreeAndNil(Stream);
// für das if-Assigned im OnDownloadComplete ... oder einfach lassen, auch wenn es eh nichts sinnvolles enthält, und nur auf Assigned(Error) prüfen
DError := AcquireExceptionObject
as Exception;
end;
TThread.Synchronize(
nil,
procedure
begin
OnDownloadComplete(
URL, Stream, DError);
end);
finally
DError.Free;
Stream.Free;
HTTP.Free;
end;
end).Start;
end;
ThreadedDownloadFile('
http://sonst.wo/dat.ei', YourDownloadComplete);
// oder
ThreadedDownloadFile('
http://sonst.wo/dat.ei',
procedure(
const URL:
String; Stream: TStream; Error:
Exception)
begin
if Assigned(Stream)
then begin // if not Assigned(Error) then
Machwas(Stream);
ShowMessage('
Download complete: ' +
URL);
end;
end);