Graza
Graza

Reputation: 5050

Constructing an Object from a Class Reference

I have a method which constructs an object, calls an Execute method, and frees the object. The type of object is determined by a TClass descendant passed into the method. Note this is Delphi for Win32 I am talking about, not .NET.

Edit: I should point out that this is Delphi 2006, as it has been noted in answers below that in future versions the NewInstance call may not be required. In my case, however, it is required. As such, I would imagine the answer to my question (is it safe? and does CreateForm() have a potential leak) would need to be answered on the basis that this is Delphi 2006

Edit#2: seems that the solutions given for D2007 & D2009 do in fact work for D2006. I must have picked up the "NewInstance" habit from an earlier version of Delphi...

function TPageClassFactory.TryExecute(ScrnClass: TCustomPageClass): boolean;
//TCustomPageClass = class of TCustomPage
var
  ScrnObj: TCustomPage; //TCustomPage defines an abstract Execute() method
begin
  Result := FALSE; //default
  ScrnObj := TCustomPage(ScrnClass.NewInstance); //instantiate
  try
    ScrnObj.Create(Self);  //NB: Create() and Execute() are *virtual* methods
    ScrnObj.Execute;       
  finally
    FreeAndNil(ScrnObj);
  end;
  Result := TRUE;
end;

What I want to know is whether this is safe - what will happen here if Create() raises an exception?

Looking at a similar example, from Forms.pas.TApplication.CreateForm(), a different approach has been taken to exception handling (I've cut out the irrelevant bits below):

procedure TApplication.CreateForm(InstanceClass: TComponentClass; var Reference);
var
  Instance: TComponent;
begin
  Instance := TComponent(InstanceClass.NewInstance);
  TComponent(Reference) := Instance;
  try
    Instance.Create(Self);
  except
    TComponent(Reference) := nil;
    raise;
  end;
end;

In the Forms.pas method, does this mean that memory is leaked when an exception occurs in the Create() method? My understanding was that InstanceClass.NewInstance allocated memory, thus in this case the memory is not being deallocated/released/freed?

Upvotes: 3

Views: 1405

Answers (4)

Toon Krijthe
Toon Krijthe

Reputation: 53366

You should put the create out of the try finally block.

But a better solution is:

type 
  TMyClass = class ()
  public
    constructor Create(...); virtual;
    function Execute: Boolean; virtual;
  end;
  TMyClassClass = class of TMyClass;


procedure CreateExecute(const AClass: TMyClassClass): Boolean;
var
  theclass : TMyClass;
begin
  theclass := AClass.Create;
  try
    Result := theclass.Execute;
  finally
    theclass.Free;
  end;
end;

Upvotes: 12

PetriW
PetriW

Reputation: 1245

Edit:

Didn't fully remember how it was in old delphi versions but apparently this should work in all based on other replies.

Note, Create has been calling Destroy on fail for as long as I can remember. It shouldn't be after I think.

Code would be:

procedure TPageClassFactory.TryExecute(ScrnClass: TCustomPageClass);
var
  ScrnObj: TCustomPage;
begin
  ScrnObj := ScrnClass.Create(Self);  // Exception here calls the destructor
  try
    ScrnObj.Execute; // Exception here means you need to free manually      
  finally
    FreeAndNil(ScrnObj); // Be free!
  end;
end;

I removed the result returned by the original function as it can never be false, only "unassigned" (exception) or true. You could after all get an exception before you assign result to false. ;)

Upvotes: 2

Rob Kennedy
Rob Kennedy

Reputation: 163247

There have been a few questions raised in comments that I'd like to clarify.

First is the continued myth that the constructor needs to be virtual. It does not. Consider this example:

type
  TBase = class
    constructor Create(x: Integer);
  end;
  TDerived = class(TBase)
    field: string;
  end;
  TMetaclass = class of TBase;

var
  instance: TBase;
  desiredClass: TMetaclass;
begin
  desiredClass := TDerived;
  instance := desiredClass.Create(23);
  Assert(instance.ClassName = 'TDerived');
  Assert(instance is TDerived);
  Assert(instance.field = '');
end;

The created object will be a full-fledged instance of class TDerived. Enough memory will have been allocated to hold the string field, which didn't exist in the base class.

There are two conditions that must be true before you'll need a virtual constructor:

  1. The constructor will be called virtually. That is, you'll have a variable of the base-class metaclass type, and it will hold a value of a derived class, and you will call a constructor on that variable. That's demonstrated in the code above. If all your constructor calls are directly on the class names themselves (i.e., TDerived.Create(23)), then there's nothing to be gained from virtual methods.
  2. A subclass of the base class will need to override the constructor to provide class-specific initialization. If all descendants use the same construction, and only vary in other methods, ten there's no need to make the constructor virtual.

What's important to realize here is that those two rules are no different from the factors that determine when the make any other method virtual. Constructors aren't special in that regard.

The constructor knows which class to construct based not on the class where the constructor was defined, but on the class the constructor was called on, and that class is always passed as a hidden first parameter for every constructor call.


Second is the issue of whether NewInstance should be called in place of or in addition to the constructor. I think other comments have already established that it has nothing to do with compatibility with older Delphi versions. All versions have supported calling constructors on class references without the need for NewInstace. Rather, the confusion comes from looking at TApplication.CreateForm and treating it as an example of how things should be done. That's a mistake.

CreateForm calls NewInstance before calling the constructor because CreateForm's primary reason for existence is to ensure that the global form variable that the IDE declares is valid during the form's own event handlers, including OnCreate, which runs as part of the constructor. If the CreateForm method had done the usual construction pattern, then the global form variable would not yet have had a valid value. Here's what you might have expected to see:

TComponent(Reference) := InstanceClass.Create(Application);

Simple and obvious, but that won't work. Reference won't get assigned a value until after the constructor returns, which is long after the form has triggered some events. If you follow good programming practice and never refer to that variable from within the form class itself, then you'll never notice. But if you follow the documentation's instructions, which are written for an inexperienced audience, then you will refer to the global form variable from within the form's own methods, so the CreateForm method does what it can to make sure it's assigned in time.

To do that, it uses a two-step construction technique. First, allocate memory and assign the reference to the global variable:

Instance := TComponent(InstanceClass.NewInstance);
TComponent(Reference) := Instance;

Next, call the constructor on the instance, passing the TApplication object as the owner:

Instance.Create(Self);

It's my opinion that CreateForm should be called exactly once in any program. I'd prefer zero times, but it has the side effect of defining Application.MainForm, which is important for other aspects of a Delphi program.


Third is the notion that it's unusual for an object to call a constructor on itself.

In fact, this happens all the time. Every time you call an inherited constructor, you're calling a constructor on an object that already exists. The inherited constructor is not allocating a new object. Likewise, the VCL has some examples of non-inherited calls of constructors. TCustomForm.Create delegates much of its construction tasks to its CreateNew constructor.

Upvotes: 4

mghie
mghie

Reputation: 32334

Re your question about memory being leaked when Create() raises an exception: You should try it out for yourself. I just did on Delphi 2007, and with your code FastMM4 shows an error dialog about the attempt to call a virtual method on an already freed object, namely Destroy(). So the exception in Create will already lead to the destructor being called and the memory being freed, so your code is actually wrong. Stick to the idiom used in the answer by Gamecat, and everything should work.

Edit:

I just tried on Delphi 4, and the behaviour is the same. Test code:

type
  TCrashComp = class(TComponent)
  public
    constructor Create(AOwner: TComponent); override;
    destructor Destroy; override;
  end;

constructor TCrashComp.Create(AOwner: TComponent);
begin
  inherited Create(AOwner);
  raise Exception.Create('foo');
end;

destructor TCrashComp.Destroy;
begin
  Beep;
  inherited Destroy;
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  C: TComponent;
begin
  C := TComponent(TCrashComp.NewInstance);
  try
    C.Create(nil);
    C.Tag := 42;
  finally
    C.Free;
  end;
end;

With FastMM4 the Free in the finally block gives the same error, because C has been freed already. On application shutdown the exception and the exception string are reported as memory leaks, though. This is however not a problem with the code, but with the runtime.

Upvotes: 2

Related Questions