HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Resumable download with System.net.HTTPClient

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withsystemhttpclientnetresumabledownload

Problem

This is a Delphi class, based on System.net.HTTPClient with a function for downloading a file from a URL and saving on a filename destination:

function Download(const ASrcUrl : string; const ADestFileName : string): Boolean;


The main feature is the ability to suspend or resume partial download.

```
unit AcHTTPClient;

interface

uses
System.Net.URLClient, System.net.HTTPClient;

type
TAcHTTPProgress = procedure(const Sender: TObject; AStartPosition : Int64; AEndPosition: Int64; AContentLength: Int64; AReadCount: Int64; ATimeStart : Int64; ATime : Int64; var Abort: Boolean) of object;
TAcHTTPClient = class
private
FOnProgress: TAcHTTPProgress;
FHTTPClient: THTTPClient;
FTimeStart: cardinal;
FCancelDownload: boolean;
FStartPosition: Int64;
FEndPosition: Int64;
FContentLength: Int64;
private
procedure SetProxySettings(AProxySettings: TProxySettings);
function GetProxySettings : TProxySettings;
procedure OnReceiveDataEvent(const Sender: TObject; AContentLength: Int64; AReadCount: Int64; var Abort: Boolean);
public
constructor Create;
destructor Destroy; override;
property ProxySettings : TProxySettings read FProxySettings write SetProxySettings;
property OnProgress : TAcHTTPProgress read FOnProgress write FOnProgress;
property CancelDownload : boolean read FCancelDownload write FCancelDownload;
function Download(const ASrcUrl : string; const ADestFileName : string): Boolean;
end;

implementation

uses
System.Classes, System.SysUtils, Winapi.Windows;

constructor TAcHTTPClient.Create;
// -----------------------------------------------------------------------------
// Constructor
begin
inherited Create;

// create an THTTPClient
FHTTPClient := THTTPClient.Create;
FHTTPClient.OnReceiveData := OnReceiveDataEvent;

// setting the timeouts
FHTTPClient.ConnectionTimeout := 5000;
FHTTPClient.Respo

Solution

Your code looks pretty good to me. Just a few quick notes:

There is an Exception.CreateFmt constuctor you can use. For example:

raise Exception.CreateFmt('Server error %d: %s', [aResponse.StatusCode, aResponse.StatusText]);


Instead of if aFileStream <> nil, the Delphi idiom is if Assigned(aFileStream):

if Assigned(aFileStream) then aFileStream.Free;


But you don't need to check. You can replace that line with just this:

aFileStream.Free;


Free is a class method. You can call it on a null reference without problems. That's how they designed it to work.

There's need to do this: aResponse := nil. The object will be destroyed when the variable goes out of scope.

function TAcHTTPClient.Download is quite long. It would be good if you could break it up into subfunctions if possible, just for the sake of readability.

The following is repeated code. That violates the Don't Repeat Yourself principle (DRY). You could wrap it in a function.

// checks if the response StatusCode is 2XX (aka OK) 
if (aResponse.StatusCode  299) then
  raise Exception.Create(Format('Server error %d: %s', [aResponse.StatusCode, aResponse.StatusText]));

Code Snippets

raise Exception.CreateFmt('Server error %d: %s', [aResponse.StatusCode, aResponse.StatusText]);
if Assigned(aFileStream) then aFileStream.Free;
aFileStream.Free;
// checks if the response StatusCode is 2XX (aka OK) 
if (aResponse.StatusCode < 200) or (aResponse.StatusCode > 299) then
  raise Exception.Create(Format('Server error %d: %s', [aResponse.StatusCode, aResponse.StatusText]));

Context

StackExchange Code Review Q#160561, answer score: 3

Revisions (0)

No revisions yet.