views:

164

answers:

6

What is the best practice for returning simple objects from functions / procedures in delphi?

eg. 2 kinds of code:

pass created object as reference, populate object in Proc, destroy it afterwards

procedure Proc(var Obj: TMyObject);
begin
  // populate Obj
end;

O := TMyObject.Create;
try
  Proc(O);
  // manipulate populated object
finally
  O.Free;
end;

or get created object as result from function, destroy after manipulation

function Func: TMyObj;
begin
  Result := TMyObj.Create;
end;

O := Func;
if O <> nil then
begin
  try
    // manipulate
  finally
    O.Free;
  end;
end;
+4  A: 

It depends on the lifetime of the object and on who is responsible for it. Most of the time objects should be created and destroyed by the same entity.

Let's say your method fills a TStringList with results from parsing a file. Should you let that function create the TStringList, or should you create it and pass as a reference?

I find it more readable to create it, pass it as reference, and later destroy, all in consecutive lines of code.

Now let's consider that you have a function that returns a TCustomer, for each customer added. In that case I would use a function, because I suppose that my entity would have a list, or something, of customers responsible for destroying them when not needed.

Mihaela
A: 

I often use the construct

FUNCTION SomeFunction(SL : TStrings = NIL) : TStrings;
  BEGIN
    IF Assigned(SL) THEN Result:=SL ELSE Result:=TStringList.Create;
    // Use Result for the remainder of the function
  END;

That way, I can use it both as a PROCEDURE with a passed-in reference, and as a FUNCTION which creates the instance itself.

HeartWare
-1 I find your practice very confusing. I would rather create two methods (one calling the other after having created the object) and make the method name clearly expressing what is going on. What if you want to call `SomeFunction (Obj) ` when `Obj` is a parameter of your own method? You can't know if it's `nil` or not, so you can't know if you have to free the returned object or not...
Smasher
I don't understand your comment "What if you want to call SomeFunction (Obj) when Obj is a parameter of your own method? You can't know if it's nil or not, so you can't know if you have to free the returned object or not..". What do you mean by "when Obj is a parameter of your own method?"???If you ould elaborate with a code example, then I'll be happy to explain it to you...
HeartWare
the thing is, that you should write methods in a way, that every one serves a single purpose, not doing more operations in one method, therefore your solution is confusing and not straightforward
Juraj Blahunka
+2  A: 

My rule is to have ownership and creation altogether. I always have the creator be the owner and thus have the responsability of destroying the object. The creation of the object is explicit in the invocation code, it is never a side effect of the invocation.

So the usual signatures of my functions are

function Func(o:tMyO): TMyO;
begin
  // ....
  Result := o;
end;

this way I may do either

   o := func(TMyO.create);

or

  o := TMyO.create;
  // ...
  func(o);
PA
+2  A: 

It is a common Delphi idiom to let the caller create the object and pass it as a parameter. Note that you don't have to declare it var in almost all cases.

procedure Proc (Obj : TMyObject)
begin
  Obj.SomeProperty := 'SomeValue';
  ...
end;

Calling Code:

Obj := TMyObject.Create;
try
  Proc (Obj);
finally
  FreeAndNil (Obj);
end;

This avoids confusion about who has to free the object. Note that if you have a chain of method calls it can quicky become very complicated to keep track of objects that need to be freed somewhere along the line.

One more drawback: having creation and destruction scattered in the code makes it impossible to use try...finally blocks, which is just another helpful idiom to avoid resource leaks.

If you want your method to create the object, I would make it explicit in the function name, something like CreateAndInitializeList sounds right to me.

Smasher
+6  A: 

There is no best practice. The primary thing you should do, though, is to make sure it's always clear who is responsible for destroying the object at any given time, even when an exception occurs.

There's nothing wrong with a function creating a new instance and returning it. Such a function is a factory. You can treat it just like a class's constructor, so you should make sure that it behaves like a constructor: Either return a valid object or throw an exception. It never returns a null reference.

function Func: TMyObj;
begin
  Result := TMyObj.Create;
  try
    Result.X := Y;
  except
    Result.Free;
    raise;
  end;
end;

That's an exception-handling pattern you don't see very often, but it's important for this style of function. Returning the object transfers ownership from the function to the caller, but only if it manages to execute completely. If it has to leave early because of an exception, it frees the object because the caller has no way to free it itself. (Functions that terminate due to an exception do not have return values.) The caller will use it like this:

O := Func;
try
  writeln(O.X);
finally
  O.Free;
end;

If there's an exception in Func then O never gets assigned, so there's nothing available for the caller to free.


When the caller creates the object and you pass it to another function to initialize it, do not make the parameter a "var" parameter. That places certain restrictions on the caller, who must use a variable of exactly the type requested by the function, even if some descendant type was created instead.

Such a function should not free the object. The caller doesn't grant ownership responsibility to the functions it calls, especially when it plans on using the object after the function returns.

Rob Kennedy
thanks for getting me straight :) great answer
Juraj Blahunka
+1  A: 

As mentioned, in general the same entity that created the object should free it and that means that the caller should create the object reference rather than having it done inside the function.

However, this is only possible if the caller knows the exact type of the item to be returned rather than a supertype. For instance:

var E: TEmployee;

E := CreateEmployee(EmployeeID);  // Could return TEmployee or subclasses TManager or TRetiredEmployee

try

    E.SendEmail(MessageText);
    if (E is TRetiredEmployee) then
        E.PrintLetter;

finally

    E.Free;

end;

In cases like this, I find it's helpful to include the word "Create", or other indicator, in the name of the factory function I'm calling.

Larry Lustig