This code
This code
vBoldSystem.StartTransaction();
try
vList := self.Clone as TBoldObjectList;
vList.SubscribeToObjectsInList := false;
vList.SubscribeToLocatorsInList := false;
for I := 0 to vList.Count - 1 do
if not vList[i].BoldObjectIsDeleted then
vList[i].Delete
finally
vList.free;
vBoldSystem.CommitTransaction();
end;
Got this warning in Delphi Berlin
[dcc32 Warning] BoldSystem.pas(8425): W1036 Variable 'vList' might not have been initialized
So I have to use two nested try finally ?
I purpose as compiler think if self.Clone raise exception vList is undefined
vBoldSystem.StartTransaction();
try
vList := self.Clone as TBoldObjectList;
vList.SubscribeToObjectsInList := false;
vList.SubscribeToLocatorsInList := false;
for I := 0 to vList.Count - 1 do
if not vList[i].BoldObjectIsDeleted then
vList[i].Delete
finally
vList.free;
vBoldSystem.CommitTransaction();
end;
Got this warning in Delphi Berlin
[dcc32 Warning] BoldSystem.pas(8425): W1036 Variable 'vList' might not have been initialized
So I have to use two nested try finally ?
I purpose as compiler think if self.Clone raise exception vList is undefined
You can add vList := nil; before try block. If Clone fails vList.Free will be called on uninitialized object reference.
ReplyDeleteDalija Prasnikar still bad: imagine that "Clone" succeeds, but "as" fails. Here we go, memory leak and potentially resources lock leak too.
ReplyDeletefinally
ReplyDelete....
vBoldSystem.CommitTransaction();
end;
what if commit fails?
usually it is done like
start
try
...
commit
except
rollback
raise
end
personally. i'd do lifetime management like that:
ReplyDeletevar vList : TBoldObjectList;
vTmp: T_Self_Clone_Type;
vTmp := self.Clone;
try
vList := vTmp as TBoldObjectList;
.....
finally
vList := nil; // just to prevent future errors
vTmp.Destroy;
vBoldSystem.CommitTransaction();
end;
Whenever I need to perform a delete to a list item I always count down:
ReplyDeletefor I := vList.Count - 1 downto 0 do
vList[i].Delete
Read more careful - that code is not deleting items from the list - that would be list.Delete(i) - but it's calling the Delete method on each item in the list.
ReplyDeleteArioch The It is Self.Clone - and then type cast... not some random object that came from who knows where... I have hard time imagining under which circumstances would that typecast fail only sometimes. It either works or not.
ReplyDeleteHopefully Embacadero one day will add Bold back to Delphi. It would probably cost them nothing as the people who bought Bold with the source code able to recompile it in Tokio without problems.
ReplyDeleteDalija Prasnikar easy. In few years some maintainer extendd the program and...
ReplyDeleteIf you are totally sure it cannot be - don't use slow AS, use fast typecast instead.
Pointer(vList) := Self.Clone;
Or less universal
vList := T_vList_Type(Self.Clone);
It would add a bonus warranty there would be no exception possibly thrown.
It is kind of the same thing why Delphi issues warning about enumeration types and CASE lacking ELSE branch. Sure, today you enlisted all the possible type values, but tomorrow it might no more be so.
Stefan Glienke well spotted, thanks!
Arioch The if commit fails the global application handler take care of it. I try to not add a lot of try except in code if not needed.
ReplyDeleteArioch The If 'as' typecast fails it leaves a trace. And it also clearly points into some refactoring error. I don't think memory leak would be much of a problem in that case.
ReplyDeleteRoland Bengtsson at least move commit before finally.
ReplyDeleteIf there is some exception unhandled, then something went unexpected way.
If so you can not be sure if you commit valid "data" or not. So, commit should only be inside finally (that is, requested to be applied to erroneous data as well) in read-only transactions
Vadim Z do you use Bold in your development?
ReplyDelete