I have a general question about best practice in OO Delphi. Currently, I put try-finally blocks anywhere I create an object to free that object after usage (to avoid memory leaks). E.g.:
aObject := TObject.Create;
try
aOBject.AProcedure();
...
finally
aObject.Free;
end;
instead of:
aObject := TObject.Create;
aObject.AProcedure();
..
aObject.Free;
Do you think it is good practice, or too much overhead? And what about the performance?
It's definitely best practice to use try-finally.
In the event of an exception being raised, that object will be freed.
As for performance: measure before you optimise.
As Frank said, "as for performance: measure before you optimize." Repeating it to emphasize.
Also, if you're creating a bunch of objects in a method, you don't need to use a try..finally block for each of them. That can lead to an ugly indentation mess. create, try, create, try, create, try, do something, finally, free, finally, free, finally, free. Ugh! Instead, you can set the object references to nil at the top of the method, then create them all, do one try block, and free them all in the finally section.
That'll save some overhead and performance, (though you'll probably never notice the difference,) but more importantly it'll make your code cleaner and easier to read while maintaining the same level of safety.
To answer part two of your question:
try finally hardly has any overhead.
In fact there are plenty of methods having an implicit try...finally block. For instance just having a function using a local var of any Interface type and assigning that a value has.
--jeroen
If you're creating the object in a class's constructor and the object will be owned by the enclosing class, you'll want to free it in the owning class's destructor.
I tend to use FreeAndNil() instead of calling Free.
EDIT: But as others have said, you definitely always want to free what you create.
Last year at Delphi Developer days I saw some of Marco Cantu's private stash of code and he calls try finally every single time he creates anything.
Somebody asked him about it, and he said he tries to do it all the time.
But it's especially a good idea for multi-threaded code when it comes to entering and exiting critical sections, although that's not on the topic, it's a good thing to remember.
Obviously sometimes it's a bit obtrusive and if it's not in the culture of your work environment to be spot-on in your robsutness it may make you look like a goodie-two-shoes. But I think it's a good idea. It's kind of like Delphi's attempt at enforced manual garbage collection.
Even if it is much recommended to do that, I won't always do it.
My rules of using or not using the try/finally:
The only place where is difficult to make a decision is when you need performance while creating the object thousands of times (in a loop for example). In this case, I don't use try/except if the object is performing simple tasks and there is a small chance to see it crashing.
In my opinion, there is only one reason an objects construction should not be followed by (or in as Mason pointed out) a try / finally block.
This management can take three forms:
With #1, when the reference has a broader scope, the reference should be set to nil immediately if it isn't constructed right away. That way when it is checked for reference you know you have an accurate reading. This is most common with member objects that are constructed as part of a larger class, and then cleaned up when the parent object is destroyed.
With #2, when an object is added to a list, you want to use a try-except block (one of the few times I use one) just in case an exception occurs after the object is constructed and before it is added to the managing list. Ideally the first line after the construction is adding it to the list, or the list is actually a factory class that gives you an object already added to the list.
With #3, when an object has another lifetime manager, you really should make sure having it managed by that manager is the right thing to do. If you are constructing a VCL control, you may be tempted to have the form (or any other control) own it, but that actually puts additional overhead on the construction and destruction. If possible you should explicitly free it, this is especially true if you put the control on once, then you know you will be freeing it in your form's destructor or when it closes. The only time you can't do this is if the control creation is more dynamic.
So yes, it is a best practice to use a lot of try / finally blocks. You should only have a few try / except blocks, and most all of them should trap very specific exception types, and / or re-raise the exception. If you have more try / except than try / finally blocks, then you are doing it wrong.
Yes it is always a good idea (essential) if the code that creates the object is responsible for free'ing it. If not, then try/finally isn't appropriate but then again neither is .Free in any case!
However, it can be cumbersome to have this boilerplate code peppering your "business logic", and you might want to consider an approach which has the same guarantee of freeing your objects but which is much cleaner (and has other benefits), such as my own AutoFree() implementation.
With AutoFree() your code could then be written:
aObject := TObject.Create;
AutoFree(@aObject);
aObject.AProcedure();
or alternatively, since the implementation uses a reference to a reference (to enable auto-NIL'ing as well as Free'ing), you can even pre-register your references that you wish to be AutoFree'd to keep such housekeeping declarations away from your business logic and keep the real "meat" of your code as clean as possible (this is especially beneficial when potentially multiple objects need to be Free'd):
AutoFree(@aObject1);
AutoFree(@aObject2);
aObject1 := TObject.Create;
aObject1.AProcedure();
// Later on aObject2 is (or may be) also created
:
Not shown in my original posting is a later addition to the mechanism to support registering multiple references in a single AutoFree() call, but I'm sure you can figure out the changes needed to support this on your own, if you wish to be able to do this:
AutoFree([@aObject1, @aObject2]);
aObject1 := TObject.Create;
aObject1.AProcedure();
// Later on aObject2 is (or may be) also created
:
if you're creating a bunch of objects have a look at this question
what-is-the-best-way-to-do-nested-try-and-finally-statement-in-delphi