Use of nested "try/finally" "try/except" statements

Question

I have seen this code posted here on StackOverflow:

with TDownloadURL.Create(nil) do
  try
    URL := 'myurltodownload.com';
    filename := 'locationtosaveto';
    try
      ExecuteTarget(nil);
    except
      result := false;
    end;
    if not FileExists(filename) then
      result := false;
  finally
    free;
  end;

Can't it be simplified to look like:

 Result:= FALSE;               <--------- Compiler complains
 DeleteFile(Dest);
 dl:= TDownloadURL.Create(NIL);
 TRY
   dl.URL:= URL;
   dl.FileName:= Dest;
   dl.ExecuteTarget(NIL);           
   Result:= FileExists(Dest);
 FINALLY
   dl.Free;
 END;

The final Result:= ... will never be executed if something went wrong in 'ExecuteTarget' because the program will jump directly to 'finally'. Right? So, the function will return FALSE. Am I doing something wrong?

---

PS:

1. I intend to use this code in a thread.
2. I just put the function in Delphi and the compiler complains about the first line: "Value assigned never used."

Joellejoellen

and where is the initiation of result at the first code? result = true? – Massive

@igon: I don't know. Probably was just demonstration/skeleton code. – Joellejoellen

Answer 1

The difference is that your second example passes exceptions back to the caller, while the original traps them and returns false. I'd characterise that style of coding as "I don't care why it failed, I only care whether it succeeded". Which can be reasonable in some cases (like trying to download an update).

So your code is very different from the original in that way - you are expecting the caller to handle exceptions that the original code does not.

Also, the compiler complaint is because there's no branch in your code - either if works and result is determined by the second assignment or you have an exception and Result is irrelevant.

Result := FALSE; //   <--------- Compiler complains
DeleteFile(Dest);
dl := TDownloadURL.Create(nil);
try
   dl.URL := URL;
   dl.FileName := Dest;
   dl.ExecuteTarget(nil);
   Result := FileExists(Dest);
finally
   dl.Free;
end;

Ardeb

Answer 2

In the original, if ExecuteTarget throws an exception, it will still test of filename exists.

In yours, if ExecuteTarget throws an exception, result is always false.

Also, unless you skipped a line, on the original, if ExecuteTarget succeeds and the file exists, result is never set.

Eiger

Sorry, I added this a line of code while you were answering to me: DeleteFile(Dest); – Joellejoellen

"In the original, if ExecuteTarget throws an exception, it will still test of filename exists" - And this is wrong because the function actually failed to download the NEW file from internet. Right? – Joellejoellen

@Altar: I am not commenting on the wisdom of the original code, mrerely reporting what it does. – Eiger

> "result is always false" - Actually, if code optimization is on and an exception is thrown, 'Result' would be unassigned. – Gorgonian

@Sertac: Regardless of whether optimization is on, 'result" is effectively undefined. Even if it is assigned in the function, since the exception escapes to the caller it will never be assigned to any LHS variable in the caller. – Plexiglas

Answer 3

The first version just eat the exception and never raise to upper callers, it treat exception as false return. For your simple version, exception will be thrown out.

Bottomless