How to handle exceptions when creating FileStream
-
08-10-2019 - |
Question
I have a function like this, that I would like to refactor
function Myfunction(sUrl, sFile: String) : Boolean;
var
GetData : TFileStream;
begin
Result := False;
//if the line below fails, I get an unhandled exception
GetData := TFileStream.Create(sFile, fmOpenWrite or fmCreate);
try
try
IdHTTP.Get(sUrl, GetData);
Result := (IdHTTP.ResponseCode = 200);
except
on E: Exception do begin
MessageBox(0, PChar(E.message), 'Niðurhala skrá', MB_ICONERROR or MB_OK);
end;
end;
finally
GetData.Free;
end;
end;
Procedure SomeOtherCode;
Begin
//How can I best defend against the unhandled exception above
//unless the call to the function is packed in a try .. except block
//the code jumps skips the if statement an goes to next
//exception block on the stack
if MyFunction('http://domain.com/file.html', 'c:\folder\file.html') then
ShowMessage('Got the file')
else
ShowMessage('Error !');
End
end;
Question:
Please refer to the comment within the procedure SomeOtherCode above.
Best Regards
Solution
Just wrap the code where you want to trap exceptions in a try..except block:
function MyFunction(...): Boolean;
var
Stream: TFileStream;
begin
Result := False;
try
Stream := TFileStream.Create(...);
try
// more code
Result := ...
finally
Stream.Free;
end;
except
// handle exception
end
end;
OTHER TIPS
The whole point about exception handling is two-fold:
finally
is for resource cleanup; you see this often in business logicexcept
is for reacting on specific exception (and getting rid of state logic through function results and intermediate variables); you hardly see it in business logic
In your case:
Myfunction
should not return a Boolean, not contain an except
block, and not perform a MessageBox
but just let the exceptions propagate.
SomeOtherCode
should contain the except
block and tell the user what went wrong.
Example:
procedure Myfunction(sUrl, sFile: String);
var
GetData: TFileStream;
begin
Result := False;
//if the line below fails, I get an unhandled exception
GetData := TFileStream.Create(sFile, fmOpenWrite or fmCreate);
try
IdHTTP.Get(sUrl, GetData);
if (IdHTTP.ResponseCode <> 200) <> then
raise Exception.CreateFmt('Download of %s failed, return code %d', [sURl, IdHTTP.ResponseCode]);
finally
GetData.Free;
end;
end;
procedure SomeOtherCode:
begin
try
MyFunction('http://domain.com/file.html', 'c:\folder\file.html');
except
on E: Exception do begin
MessageBox(0, PChar(E.message), 'Niðurhala skrá', MB_ICONERROR or MB_OK);
end;
end;
end;
Now the code is much cleaner:
- no more UI in your business logic
- one place where your
except
is being handled - all failures are handled equally (
cannot create file
,download failure
)
Good luck with this.
--jeroen
One solution which is quite popular is to avoid 'success' or 'failure' return values completely. Instead of a function, use a procedure and handle failures using exceptions instead:
procedure Download(sUrl, sFile: String);
and then
try
Download ('http://domain.com/file.html', 'c:\folder\file.html');
ShowMessage('Got the file')
except
on E:Exxx do
begin
// handle exception
ShowMessage('Error !');
end
end;
This has also the effect that nobody can invoke the function and silently ignore the return value.
If you want your function to show messages to the user and return false on any failure, code it as follows:
function Myfunction(sUrl, sFile: String) : Boolean;
var
GetData : TFileStream;
begin
Result := False;
try
//if the line below fails, I get an unhandled exception
GetData := TFileStream.Create(sFile, fmOpenWrite or fmCreate);
try
try
IdHTTP.Get(sUrl, GetData);
Result := (IdHTTP.ResponseCode = 200);
except
on E: Exception do begin
MessageBox(0, PChar(E.message), 'Niðurhala skrá', MB_ICONERROR or MB_OK);
end;
end;
finally
GetData.Free;
end;
except
// you can handle specific exceptions (like file creation errors) or any exception here
end;
end;
Warning IMHO this design is mixing business logic (such as get a resource/file from the Internet and save it to a file) and user interface logic (such as showing messages to the user in case of errors).
In general, is a better approach to separate business from UI logic, because your code is reusable.
For example you might want to re-factor as this:
function DownloadToAFile(const sUrl, sFile: string): boolean;
var
GetData : TFileStream;
begin
GetData := TFileStream.Create(sFile, fmOpenWrite or fmCreate);
try
IdHTTP.Get(sUrl, GetData);
Result := (IdHTTP.ResponseCode = 200);
finally
GetData.Free;
end;
end;
function UIDownloadToAFile(const sUrl, sFile: string): boolean;
begin
try
Result := DownloadToAFile(sURL, sFile);
except
on E: EIDException do //IndyError
MessageBox(0, PChar(E.message), 'Internet Error', MB_ICONERROR or MB_OK);
on E: EFileCreateError do //just can't remember the extact class name for this error
MessageBox(0, PChar(E.message), 'File create Error', MB_ICONERROR or MB_OK);
end;
end;
procedure SomeOtherCode:
begin
if UIDownloadToAFile('http://domain.com/file.html', 'c:\folder\file.html') then
ShowMessage('Got the file')
else
ShowMessage('Error !');
end;
Tomorrow, if you're writing a service, or a DataSnap module, you're free to use the DownloadToAFile or maybe to write a new ServiceDownloadToAFile wich in turns writes errors to a log or windows events, or maybe send a email notifying the HostAdmin about it.
You should use only one try
and get in this your all function code.
For some reason most people misuse except-finally combination. Correct sequence is
try
// allocate resource here
try
finally
// free resource here
end;
except
// handle exception here
end;
This lets you catch exceptions in constructor and destructor.