Why not use the Result variable directly when returning object?

Question

I've seen in a lot of examples creating a variable with the same type as the Result and assigning to it at the end of the function instead of just using the Result variable in the first place.

For instance in a code inside System.JSON

class function TJSONObject.ParseJSONValue(
  const Data: TArray<Byte>; 
  const Offset: Integer; 
  const ALength: Integer; 
  Options: TJSONParseOptions
): TJSONValue;
var
  Parent: TJSONArray;
  Answer: TJSONValue;
  Br: TJSONByteReader;
begin
  Parent := TJSONArray.Create;
  Answer := nil; { Why not just use Result directly here? }
  Br := TJSONByteReader.Create(
          Data, 
          Offset, 
          ALength, 
          TJSONParseOption.IsUTF8 in Options
  );
  try
    ConsumeWhitespaces(Br);
    if 
      (ParseValue(Br, Parent, TJSONParseOption.UseBool in Options) = ALength) 
      and
      (Parent.Count = 1)
    then
      Answer := Parent.Pop; { Why not just use Result directly here? }
    Result := Answer; 
  finally
    Parent.Free;
    Br.Free;
  end;
end;

Why create the variable Answer instead of just using Result?

Is this just the way the programmer decided to do it or is there a reason behind it?

Too many java programmers hired, not familiar with Object Pascal syntax? — LU RD
– LU RD, Commented Apr 17, 2018 at 20:18
Either approach works fine, it is just a matter of coding style. You can certainly use the Result directly if you want to. Either way, just be sure to call Free on the output object if an error occurs before the function exits, or you will have a memory leak (the above code does not have that protection - shame on Embarcadero for that!). — Remy Lebeau
– Remy Lebeau, Commented Apr 17, 2018 at 20:55
@RemyLebeau How could this leak? There would have to be an exception after Answer := Parent.Pop. It is safe to assume that there isn't. Of course, Parent could be leaked quote easily. — David Heffernan
– David Heffernan, Commented Apr 17, 2018 at 21:18
@DavidHeffernan: Parent is leaked if TJSONByteReader.Create raises an exception. — Remy Lebeau
– Remy Lebeau, Commented Apr 17, 2018 at 21:21
@DavidHeffernan in THIS example, the output object is not leaked, yes. But IN GENERAL, if you create an object for output, and then an exception is raised before the function exits, make sure to destroy the output object or it will be leaked. That is what my original comment was trying to state. — Remy Lebeau
– Remy Lebeau, Commented Apr 17, 2018 at 21:26

David Heffernan · Accepted Answer · 2018-04-18 06:43:05Z

Is this just the way the programmer decided to do it or is there a reason behind it?

There is no good reason for using an extra local variable here. Doing so just adds complexity. I would write it like this:

class function TJSONObject.ParseJSONValue(
  const Data: TArray<Byte>; 
  const Offset: Integer; 
  const ALength: Integer; 
  Options: TJSONParseOptions
): TJSONValue;
var
  Parent: TJSONArray;
  Br: TJSONByteReader;
begin
  Parent := TJSONArray.Create;
  try
    Br := TJSONByteReader.Create(
      Data, 
      Offset, 
      ALength, 
      TJSONParseOption.IsUTF8 in Options
    );
    try
      ConsumeWhitespaces(Br);
      if (ParseValue(Br, Parent, TJSONParseOption.UseBool in Options) = ALength)
      and (Parent.Count = 1) then
        Result := Parent.Pop
      else
        Result := nil; 
    finally
      Br.Free;
    end;
  finally
    Parent.Free;
  end:
end;

This also corrects the lifetime management and a potential memory leak, as discussed in comments.

Collectives™ on Stack Overflow

Why not use the Result variable directly when returning object?

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related