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?
-
You should put the create out of the try finally block.
But a better solution is:
type TMyClass = class () public constructor Create(...); virtual; fucntion 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;
Graza : I am using Delphi 2006 - my understanding of why is a little hazy, but NewInstance is required either because AClass.Create() does not allocate memory, or because it creates an instance of the base class type, rather than the given class type. As a result, I think create needs to be *in* the tryUlrich Gerhardt : I'm using D2006 too and happily use Create on class type references without NewInstance. And frankly, I would be surprised if this was any different in, say, D5.Gamecat : Yup, there is no difference between classname.create and classtypevar.create. Both create an instance and call the constructor.Craig Stuntz : Graza, you are wrong. Gamecat's solution works in every version of Delphi I can remember. Back before D4, anyway. You are also wrong about putting Create in the try. See Allen Bauer's blog for why.Graza : If Create() is virtual, and AClass refers to a class that is a *descendant* of TMyClass (eg TMySubClass = class(TMyClass), and then CreateExecute is called with TMySubClass as the argument), will calling AClass.Create() create a TMyClass object, or a TMySubClass object?Graza : Actually, I just tested and it appears that the correct class is indeed created. I wonder why they do it this way in CreateForm()?Craig Stuntz : Graza, yes, *if* the constructor is virtual. (As with Gamecat's example.)Graza : Constructor is virtual. I've now gone from the test apps used to check doing it *without* NewInstance, into production code, and all is working. The ways suggested here were always how I'd instinctually have done it, but something in past projects led me to the NewInstance route for some reason.DiGi : NewInstance is used in VCL's TApplication to create Forms... Check .dpr file ;)Graza : I'm pretty sure I used CreateForm() as an example some time in the past because I couldn't get the more "straightforward" code working for some reason. I'm a little baffled at this point though, which CreateForm() uses NewInstance (or at least why it still uses it) -
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. ;)
Graza : Delphi 2006 - can't use Gamecat's solution & need to use NewInstance. My understanding is that because of this, Create() is not allocating memory, thus can (and should) be inside the try..finally.mghie : The point is that (at least under Delphi 2007) the raised exception in the constructor leads to the destructor being called "behind-the scene", so doing it again in the finally block is an error.PetriW : I've only messed around with Create in this manner in later versions and you are indeed correct in that the destructor is called if Create fails. How it is for very early delphis I don't know, the poster did not specify a version.Graza : Ah, I actually had some code above the creation in my original code, which *can* result in FALSE, but didn't think to remove the result from the "cut-down" version I postedGraza : Just tested and indeed the destructor is called even if Create is called in this way. Which I think is a bit strange... an object can call its own create method after construction, which is *not* reconstructing it, so why is Destroy called? No problem though - you've given me a better way to do thisRob Kennedy : Destructors have *always* been called upon construction failure. And NewInstance has *never* been required in place of a bona-fide constructor call, for all versions of Delphi, even the first one. If your experience is otherwise, then you were doing something wrong. -
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.
Graza : Using BDS2006 - does this apply (destroy called automatically) when Create() is being called in the way I am calling it (in this case it is not used as a "constructor" and does not allocate memory for the object, but is simply a method which initialises things). (AObject.Create <> TObject.Create)mghie : If Create() overrides the method in the base class it will work. If not you should get a warning about hiding a method, and it probably will not work. Not that you would do that...mghie : Actually, I don't understand the need for NewInstance, at all. I have never used it in similar places.PetriW : NewInstance is used in a lot of examples, maybe it's a remnant from an early Delphi or C++ Builder and everyone since has just copied. ;)mghie : This is kind of weird, as even the Delphi 1 help says not to call NewInstance as it will be called automatically by the constructor. I wonder what this is all about. -
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:
- 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. - 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 forNewInstace
. Rather, the confusion comes from looking atTApplication.CreateForm
and treating it as an example of how things should be done. That's a mistake.CreateForm
callsNewInstance
before calling the constructor becauseCreateForm
'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, includingOnCreate
, which runs as part of the constructor. If theCreateForm
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 theCreateForm
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 definingApplication.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 itsCreateNew
constructor.PetriW : Thanks for the extra info! :) Good read.Graza : Brilliant! Re CreateForm(): That makes perfect sense. Really, it sounds like a workaround due to the fact that the default project templates encourage the use of Global variables (which I've always thought was naughty of Borland!)Graza : Re #3: Probably its best that some sort of "InitMembers" method is declared instead, but if for example an object calls its own Create method long after it has been created, in order to reinitialise itself, would it be freed at that point if an exception occurred?Rob Kennedy : No, Graza, the destructor won't get called in that case. Constructors know whether they're supposed to do allocation before they initialize. If they're just initializing, they don't set up a stack frame to make the destructor get called.Graza : Good to know. Then it still seems a bit odd that in the case of Application.CreateForm() that the destructor gets called on exception, as in this case Create() is not allocating memory, InitInstance() is. But I'm now steering clear of that way of doing things, so I'll happily leave that unexplained - 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.,
0 comments:
Post a Comment