This question is a continuance of a particular comment from people on stackoverflow which I've seen a few different times now. I, along with the developer who taught me Delphi, in order to keep things safe, have always put a check if assigned() before freeing objects, and before doing other various things. However, I'm now told that I should not be adding this check. I'd like to know if there is any difference in how the application compiles/runs if I do this, or if it won't affect the result at all...
if assigned(SomeObject) then SomeObject.Free;
Let's say I have a form, and I'm creating a bitmap object in the background upon the form's creation, and freeing it when I'm done with it. Now I guess my problem is I got too used to putting this check on a lot of my code when I'm trying to access objects which might potentially have been free'd at some point. I've been using it even when it's not necessary. I like to be thorough...
unit Unit1;
interface
uses
Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
Dialogs;
type
TForm1 = class(TForm)
procedure FormCreate(Sender: TObject);
procedure FormDestroy(Sender: TObject);
private
FBitmap: TBitmap;
public
function LoadBitmap(const Filename: String): Bool;
property Bitmap: TBitmap read FBitmap;
end;
var
Form1: TForm1;
implementation
{$R *.dfm}
procedure TForm1.FormCreate(Sender: TObject);
begin
FBitmap:= TBitmap.Create;
LoadBitmap('C:\Some Sample Bitmap.bmp');
end;
procedure TForm1.FormDestroy(Sender: TObject);
begin
if assigned(FBitmap) then begin //<-----
//Do some routine to close file
FBitmap.Free;
end;
end;
function TForm1.LoadBitmap(const Filename: String): Bool;
var
EM: String;
function CheckFile: Bool;
begin
Result:= False;
//Check validity of file, return True if valid bitmap, etc.
end;
begin
Result:= False;
EM:= '';
if assigned(FBitmap) then begin //<-----
if FileExists(Filename) then begin
if CheckFile then begin
try
FBitmap.LoadFromFile(Filename);
except
on e: exception do begin
EM:= EM + 'Failure loading bitmap: ' + e.Message + #10;
end;
end;
end else begin
EM:= EM + 'Specified file is not a valid bitmap.' + #10;
end;
end else begin
EM:= EM + 'Specified filename does not exist.' + #10;
end;
end else begin
EM:= EM + 'Bitmap object is not assigned.' + #10;
end;
if EM <> '' then begin
raise Exception.Create('Failed to load bitmap: ' + #10 + EM);
end;
end;
end.
Now let's say I'm introducing a new custom list object called TMyList of TMyListItem. For each item in this list, of course I have to create/free each item object. There's a few different ways of creating an item, as well as a few different ways of destroying an item (Add/Delete being the most common). I'm sure it's a very good practice to put this protection here...
procedure TMyList.Delete(const Index: Integer);
var
I: TMyListItem;
begin
if (Index >= 0) and (Index < FItems.Count) then begin
I:= TMyListItem(FItems.Objects[Index]);
if assigned(I) then begin //<-----
if I <> nil then begin
I.DoSomethingBeforeFreeing('Some Param');
I.Free;
end;
end;
FItems.Delete(Index);
end else begin
raise Exception.Create('My object index out of bounds ('+IntToStr(Index)+')');
end;
end;
In many scenarios, at least I would hope that the object is still created before I try to free it. But you never know what slips might happen in the future where an object gets free'd before it's supposed to. I've always used this check, but now I'm being told I shouldn't, and I still don't understand why.
EDIT
Here's an example to try to explain to you why I have a habit of doing this:
procedure TForm1.FormDestroy(Sender: TObject);
begin
SomeCreatedObject.Free;
if SomeCreatedObject = nil then
ShowMessage('Object is nil')
else
ShowMessage('Object is not nil');
end;
My point is that if SomeCreatedObject <> nil is not the same as if Assigned(SomeCreatedObject) because after freeing SomeCreatedObject, it does not evaluate to nil. So both checks should be necessary.
assigned(I)differ fromI <> nil? (Note that I do not use Delphi at all :p~) – ShirtwaistAssignedis exactly the same as<> nilin most cases. The exception is events, whereAssignedhas a bit of black magic to work around issues that could otherwise arise in the form designer (so you always need to useAssignedto check whether an event is assigned, whereas for anything elseAssignedand<> nilare equivalent). – HippocampusFis a function variable returning a pointer,Assigned(F)checks whetherFitself isnil, whereasF <> nilchecksF's result. – Delainedelaineyprocedure of objectis stored as eight bytes (object pointer + code pointer), andAssignedonly checks one of those two pointers, I don't recall which. That way, the form designer can use the other four bytes to store its own encoded data to keep track of the event handler, without screwing up the component's code that's running inside the IDE. – HippocampusX <> nilis rejected by the compiler ifXis an procedure of object pointer, andAssigned(X)checks even less: only two bytes ofX. – Delainedelaineyif Assigned(P) thenproducescmp dword ptr [ebp-$08],$00for me (XE/32bit). Its not real function but compiler intrinsic – Disavowaldwordcomparison for normal pointers andwordfor method pointer fields. Now i'm curious why. – Disavowalassigned(), which would be practically double or even triple code. But I was hoping for more of an answer regarding does it hurt my app or not if I have this protection. – CurrishAssigned()when not needed?" – Currishnilactually meant. It merely means nothing. Absent. Void. Not even empty. Beyond empty. Just... a black hole.nilis the equivalent of wanting to make a peanut butter and jelly sandwich, except there's no peanut butter. Not even an empty peanut butter jar. Just not even a jar. Just a jelly sandwich. – Currishthing <> nilworks whereAssigned(thing)fails. For exampleGetActiveConnection() <> nilcompiles, whileAssigned(GetActiveConnection())does not. – Cervix