Why is using procedures to create objects preferred over functions?

Question

This is similar to this question. I asked "Why?" to the most popular response but I don't know that anyone would ever look at it again. At least not in any timely manner.

Anyway, my question is about best practices for delegating responsibility for creation of objects to functions or procedures, without causing memory leaks. It seems that this:

procedure FillObject(MyObject: TMyObject; SomeParam: Integer);
begin
  //Database operations to fill object
end;

procedure CallUsingProcedure();
var
  MyObject: TMyObject;
begin
  MyObject = TMyObject.Create();
  try
    FillObject(MyObject, 1);
    //use object
  finally
    MyObject.Free();
  end;
end;

is preferred over this:

function CreateMyObject(DBID: Integer): TMyObject;
begin
  Result := TMyObject.Create();
  try
    //Database operations to fill object
  except on E: Exception do
    begin
      Result.Free();
      raise;
    end;
  end;
end;

procedure CallUsingFunction();
var
  MyObject: TMyObject;
begin
  MyObject = CreateMyObject(1);
  try
    //use object
  finally
    MyObject.Free();
  end;
end;

Why?

I'm relatively new to Delphi, having previously worked most with Java and PHP, as well as C++, though to a lesser extent. Intuitively, I lean toward the function method because:

• It encapsulates the object creation code in the function, rather than create the object separately whenever I want to use the procedure.
• I dislike methods that alter their parameters. It's often left undocumented and can make tracing bugs more difficult.
• Vague, but admittedly it just "smells" bad to me.

I'm not saying I'm right. I just want to understand why the community chooses this method and if there is good reason for me to change.

Edit: References to @E-Rock in comments are to me(Eric G). I changed my display name.

Answer 1

One problem is what Ken White wrote: you hand the user of the function an object he or she must free.

Another advantage of procedures is that you can pass several objects of a hierarchy, while a function that creates such an object always generates the same. E.g.

procedure PopulateStrings(Strings: TStrings);

To that procedure, you can pass any kind of TStrings, be it the Lines of a TMemo, the Items of a TListBox or TComboBox or a simple standalone TStringList. If you have a function:

function CreateStrings: TStrings;

You always get the same kind of object back (which object exactly is not known, as TStrings is abstract, so you probably get a TStringList), and must Assign() the contents to the TStrings you want to modify. The procedure is to be preferred, IMO.

Additionally, if you are the author of the function, you can't control whether the object you create is freed, or when. If you write a procedure, that problem is taken off your hands, since the user provides the object, and its lifetime is none of your concern. And you don't have to know the exact type of the object, it must just be of the class or a descendant of the parameter. IOW, it is also much better for the author of the function.

It is IMO seldom a good idea to return an object from a function, for all the reasons given. A procedure that only modifies the object has no dependency on the object and creates no dependency for the user.

FWIW, Another problem is if you do that from a DLL. The object returned uses the memory manager of the DLL, and also the VMT to which it points is in the DLL. That means that code that uses as or is in the user code does not work properly (since is and as use the VMT pointer to check for class identity). If the user must pass an object of his, to a procedure, that problem does not arise.

Update

As others commented, passing an object to a DLL is not a good idea either. Non-virtual functions will call the functions inside the DLL and use its memory manager, which can cause troubles too. And is and as will not work properly inside the DLL either. So simply don't pass objects into or out of a DLL. That goes with the maxime that DLLs should only use POD type parameters (or compound types -- arrays, records -- that only contain POD types) or COM interfaces. The COM interfaces should also only use the same kind of parameters.

Answer 2

Creating the object instance and passing it into another procedure makes it clear which code is responsible for freeing the instance.

In the first case (using a procedure to fill it):

MyObj := TMyObject.Create;
try
  // Do whatever with MyObj
finally
  MyObj.Free;
end;

This is clear that this block of code is responsible for freeing MyObj when it's finished being used.

MyObj := CreateMyObject(DBID);

What code is supposed to free it? When can you safely free it? Who is responsible for exception handling? How do you know (as a user of someone else's code)?

As a general rule, you should create, use, and free object instances where they're needed. This makes your code easier to maintain, and definitely makes it easier for someone who comes along later and has to try and figure it out. :)

Answer 3

I use a combination of both idioms. Pass the object as an optional parameter and if not passed, create the object. And in either case return the object as the function result.

This technique has (1) the flexibility of the creation of the object inside of the called function, and (2) the caller control of the caller passing the object as a parameter. Control in two meanings: control in the real type of the object being used, and control about the moment when to free the object.

This simple piece of code exemplifies this idiom.

function MakeList(aList:TStrings = nil):TStrings;
 var s:TStrings;
 begin
   s:=aList;
   if s=nil then 
     s:=TSTringList.Create;
   s.Add('Adam');
   s.Add('Eva');
   result:=s;
 end;

And here are three different ways to use it

simplest usage, for quick and dirty code

var sl1,sl2,sl3:TStrings;
sl1:=MakeList;

when programmer wants to make more explicit ownership and/or use a custom type

sl2:=MakeList(TMyStringsList.create);

when the object is previously created

sl3:=TMyStringList.Create;
....
MakeList(sl3);