given this code:
given this code:
var
LItem: TListItem;
[...]
LItem := nil; // * here *
Folder := GetFolder(tvFolders.Selected);
ListView.Items.BeginUpdate;
try
mFile := TGXFile.Create(Folder);
try
LItem := ListView.Items.Add;
FileToListItem(mFile, LItem);
except
on E: Exception do
begin
FreeAndNil(mFile);
raise;
end;
end;
finally
ListView.Items.EndUpdate;
end;
if Assigned(LItem) then
begin
ListView.Selected := nil;
ListView.Selected := LItem; //FI:W508 - Assignment has side effects
ListView.ItemFocused := LItem;
end;
Should the compiler really emit a hint H2077: Value assigned to 'LItem' never used in the line marked above?
var
LItem: TListItem;
[...]
LItem := nil; // * here *
Folder := GetFolder(tvFolders.Selected);
ListView.Items.BeginUpdate;
try
mFile := TGXFile.Create(Folder);
try
LItem := ListView.Items.Add;
FileToListItem(mFile, LItem);
except
on E: Exception do
begin
FreeAndNil(mFile);
raise;
end;
end;
finally
ListView.Items.EndUpdate;
end;
if Assigned(LItem) then
begin
ListView.Selected := nil;
ListView.Selected := LItem; //FI:W508 - Assignment has side effects
ListView.ItemFocused := LItem;
end;
Should the compiler really emit a hint H2077: Value assigned to 'LItem' never used in the line marked above?
Why not? Looks ok for me.
ReplyDeleteI think so, since both assignments to LItem will always execute without conditions (there is nothing between them which would jump out and skip the second one), so the first one seems to be superfluous.
ReplyDeleteYes this is a good hint. Previously the compiler was wrongly ignoring it, and create other unexpected hints instead.
ReplyDeleteYes, the code cannot reach the if statement without passing the assigment inside the try except.
ReplyDeleteFWIW the except should be changed to finally because you are raising in any case (unless there is other code in there that you did not show here).
Stefan Glienke there is more code. But you might be right anyway. I'll have to check.
ReplyDeleteCompiler is right.
ReplyDeleteOff-topic question: in the above code is it necessary to use "on E: Exception"? Wouldn't "raise" work anyway?
ReplyDeleteIt's needless to pick out only Exception. After all, you can throw things that aren't derived from Exception.
ReplyDeleteYes, if you do not access the Exception instance E in any way, you do not need it. Also as David says you might miss an exception object based on something else (or not based at all if that's semantically possible).
ReplyDelete