59

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.

Improve this question
24
  • How does assigned(I) differ from I <> nil? (Note that I do not use Delphi at all :p~) Commented Dec 18, 2011 at 0:09
  • 17
    @pst, Assigned is exactly the same as <> nil in most cases. The exception is events, where Assigned has a bit of black magic to work around issues that could otherwise arise in the form designer (so you always need to use Assigned to check whether an event is assigned, whereas for anything else Assigned and <> nil are equivalent). Commented Dec 18, 2011 at 0:14
  • 16
    No, they usually mean the same thing. The only difference is that if F is a function variable returning a pointer, Assigned(F) checks whether F itself is nil, whereas F <> nil checks F's result. Commented Dec 18, 2011 at 0:15
  • 1
    @JerryDodge, the example in your edit doesn't actually explain anything. What is it you're trying to do? Commented Dec 18, 2011 at 0:33
  • 3
    @Jerry Dodge - Also consider using FreeAndNil() instead of Free. It will help you a lot!!!! Commented Jun 19, 2013 at 16:50

5 Answers 5

Reset to default
141

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:

  1. Assigned is equivalent to testing <> nil.
  2. 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.

Improve this answer
Sign up to request clarification or add additional context in comments.

21 Comments

@David, in the TMyObject.Destroy you are calling FLogger.Free without checking if it is Assigned. is that because TMyObject.Create will always initialize it to nil when UseLogging is False? when declaring a local TObject variable in a procedure, we cannot simply call object.Free without first initializing it. or am I wrong?
@kobik You are 100% correct. An instance of a class is guaranteed to be zero initialized. Local variables are uninitialized.
@David Heffernan. Yes, I considered adding "which can be very confusing". But the point is that the answer is wrong: you can call the example .update on the example tmyobject or an object declared but not created as tmyobject, and since it tests IsAssigned, the operation of .update is defined, not "anyone's guess".
@David Heffernan Not that I want to bang on about it, but the answer still reads "you simply cannot call TMyObject.Update unless the constructor of TMyObject succeeded". I take your point that you assumed a declaration of FSettings that did not imply FSettings was NIL: since the question is about the use of IsAssigned, I made the opposite assumption.
David Heffernan is a living wikipedia :) Thanks for this anser
|
23

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.

Improve this answer

23 Comments

Unless you're working with a delegate type (procedure of object or function of object), Assigned is exactly the same as checking for <> nil.
Well of course. If you do x.Free, then x still points to the memory address where the object used to be, and both x <> nil and Assigned(x) will return True. So if your variable isn't going out of scope right away, then it's a good habit to set it to nil when you free the object it's pointing to. That's why FreeAndNil was invented.
What on earth did I say that you're twisting to mean "calling x.Free will change the x variable to point to nil"? I've clearly and specifically said the opposite.
@JerryDodge: It sounds like you're somehow convinced that Assigned(I) has some magic ability to check whether I points to an object that's already been freed. It doesn't. Like we keep telling you, Assigned checks for nil. Try it. I := TObject.Create; I.Free; if I <> nil then ShowMessage('I <> nil'); if Assigned(I) then ShowMessage('Assigned(I)'); will show two messages: I <> nil and Assigned(I). That's because both checks do the exact same thing.
@Jerry - Ctrl+Click on Assigned in Joe's test code, see if it takes you to system unit.
|
22

Why you shouldn't call

if Assigned(SomeObject) then SomeObject.Free;

Simply because you would execute something like this

if Assigned(SomeObject) then if Assigned(SomeObject) then SomeObject.Destroy;

If you call just SomeObject.Free; then it's just

 if Assigned(SomeObject) then SomeObject.Destroy;

To your update, if you afraid of the object instance reference use FreeAndNil. It will destroy and dereference your object

FreeAndNil(SomeObject);

It's similar as if you call

SomeObject.Free; SomeObject := nil;
Improve this answer

1 Comment

+10 for this!! The whole discussion under this topic comes down to the following point: SomeObject.Free destroys the instance by calling the destructor chain and releasing the allocated memory. It does not change the value of SomeObject. Since it is a pointer, i.e. a memory address, it will still bear that very same address after SomeObject.Free Moreover, AFAIK, the freed memory is not filled with null bytes as TObject.InitInstance does upon construction. So, ideally, use FreeAndNil( SomeObject ) otherwise there's no chance to tell living instance vars from dead ones
-1

I create a simple example showing my procedure:

unit Unit1; interface uses System.SysUtils, System.Types, System.UITypes, System.Classes, System.Variants, FMX.Types, FMX.Controls, FMX.Forms, FMX.Graphics, FMX.Dialogs, FMX.Controls.Presentation, FMX.StdCtrls, FMX.Objects; type TForm1 = class(TForm) Button1: TButton; Button2: TButton; procedure Button1Click(Sender: TObject); procedure Button2Click(Sender: TObject); private { Private declarations } public { Public declarations } end; var Form1: TForm1; Cir : TCircle; implementation {$R *.fmx} procedure TForm1.Button1Click(Sender: TObject); begin if Assigned(Cir) then begin Cir.Free; Cir := Nil; end; Cir := TCircle.Create(nil); with Cir do begin Parent := Form1; Height := 50; Width := 50; Position.X := 100; Position.Y := 100; end; end; procedure TForm1.Button2Click(Sender: TObject); begin if Assigned(Cir) then begin Cir.Free; Cir := Nil; end; end; end.
Improve this answer

1 Comment

How does this answer the question? Are you trying to ask a new question?
-4

I'm not completely sure of that, but it seems:

if assigned(object.owner) then object.free

works fine. In this example it would be 

if assigned(FBitmap.owner) then FBitmap.free
Improve this answer

2 Comments

no it still doesn't work. I mean sometimes it works sometimes it doesn't... as above.
but additionally testing "object <> nil" - also hasnt sorted out the problem... I prefer using another variable fex. "ifcreated_object : boolean". Well, it works fine.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.