Why should I not use "if Assigned()" before accessing objects?
Free
has some special logic: it checks to see whether Self
is nil
, and if so, it returns without doing anything -- so you can safely call X.Free
even if X
is nil
. This is important when you're writing destructors -- David has more details in his answer.
You can look at the source code for Free
to see how it works. I don't have the Delphi source handy, but it's something like this:
procedure TObject.Free;
begin
if Self <> nil then
Destroy;
end;
Or, if you prefer, you could think of it as the equivalent code using Assigned
:
procedure TObject.Free;
begin
if Assigned(Self) then
Destroy;
end;
You can write your own methods that check for if Self <> nil
, as long as they're static (i.e., not virtual
or dynamic
) instance methods (thanks to David Heffernan for the documentation link). But in the Delphi library, Free
is the only method I know of that uses this trick.
So you don't need to check to see if the variable is Assigned
before you call Free
; it already does that for you. That's actually why the recommendation is to call Free
rather than calling Destroy
directly: if you called Destroy on a nil
reference, you would get an access violation.
This is a very broad question with many different angles.
The meaning of the Assigned
function
Much of the code in your question betrays an incorrect understanding of the Assigned
function. The documentation states this:
Tests for a nil (unassigned) pointer or procedural variable.
Use Assigned to determine whether the pointer or the procedure referenced by P is nil. P must be a variable reference of a pointer or procedural type.
Assigned(P) corresponds to the test P <> nil for a pointer variable, and @P <> nil for a procedural variable.
Assigned returns False if P is nil, True otherwise.
Tip: When testing object events and procedures for assignment, you cannot test for nil, and using Assigned is the right way.
....
Note: Assigned cannot detect a dangling pointer--that is, one that is not nil, but that no longer points to valid data.
The meaning of Assigned
differs for pointer and procedural variables. In the rest of this answer we will consider pointer variables only, since that is the context of the question. Note that an object reference is implemented as a pointer variable.
The key points to take from the documentation are that, for pointer variables:
Assigned
is equivalent to testing<> nil
.Assigned
cannot detect whether the pointer or object reference is valid or not.
What this means in the context of this question is that
if obj<>nil
and
if Assigned(obj)
are completely interchangeable.
Testing Assigned
before calling Free
The implementation of TObject.Free
is very special.
procedure TObject.Free;
begin
if Self <> nil then
Destroy;
end;
This allows you to call Free
on an object reference that is nil
and doing so has no effect. For what it is worth, I am aware of no other place in the RTL/VCL where such a trick is used.
The reason why you would want to allow Free
to be called on a nil
object reference stems from the way constructors and destructors operate in Delphi.
When an exception is raised in a constructor, the destructor is called. This is done in order to deallocate any resources that were allocated in that part of the constructor that succeeded. If Free
was not implemented as it is then destructors would have to look like this:
if obj1 <> nil then
obj1.Free;
if obj2 <> nil then
obj2.Free;
if obj3 <> nil then
obj3.Free;
....
The next piece of the jigsaw is that Delphi constructors initialise the instance memory to zero. This means that any unassigned object reference fields are nil
.
Put this all together and the destructor code now becomes
obj1.Free;
obj2.Free;
obj3.Free;
....
You should choose the latter option because it is much more readable.
There is one scenario where you need to test if the reference is assigned in a destructor. If you need to call any method on the object before destroying it then clearly you must guard against the possibility of it being nil
. So this code would run the risk of an AV if it appeared in a destructor:
FSettings.Save;
FSettings.Free;
Instead you write
if Assigned(FSettings) then
begin
FSettings.Save;
FSettings.Free;
end;
Testing Assigned
outside a destructor
You also talk about writing defensive code outside a destructor. For example:
constructor TMyObject.Create;
begin
inherited;
FSettings := TSettings.Create;
end;
destructor TMyObject.Destroy;
begin
FSettings.Free;
inherited;
end;
procedure TMyObject.Update;
begin
if Assigned(FSettings) then
FSettings.Update;
end;
In this situation there is again no need to test Assigned
in TMyObject.Update
. The reason being that you simply cannot call TMyObject.Update
unless the constructor of TMyObject
succeeded. And if the constructor of TMyObject
succeeded then you know for sure that FSettings
was assigned. So again you make your code much less readable and harder to maintain by putting in spurious calls to Assigned
.
There is a scenario where you need to write if Assigned
and that is where the existence of the object in question is optional. For example
constructor TMyObject.Create(UseLogging: Boolean);
begin
inherited Create;
if UseLogging then
FLogger := TLogger.Create;
end;
destructor TMyObject.Destroy;
begin
FLogger.Free;
inherited;
end;
procedure TMyObject.FlushLog;
begin
if Assigned(FLogger) then
FLogger.Flush;
end;
In this scenario the class supports two modes of operation, with and without logging. The decision is taken at construction time and any methods which refer to the logging object must test for its existence.
This not uncommon form of code makes it even more important that you don't use spurious calls to Assigned
for non-optional objects. When you see if Assigned(FLogger)
in code that should be a clear indication to you that the class can operate normally with FLogger
not in existence. If you spray gratuitous calls to Assigned
around your code then you lose the ability to tell at a glance whether or not an object should always exist.