tags:

views:

495

answers:

3

Ok, I have the following function:

function TfPackagedItemEdit.GetRTFDescription: TStringList;
begin
  Result.Text := richDescription.Lines.Text; //stringlist
end;

The compiler generates the following warning about this line:

[DCC Warning] W1035 Return value of function 'GetRTFDescription' might be undefined

Any ideas on how I can clear up this warning? (other than just turning it off in the project options)

I've tried:

function TfPackagedItemEdit.GetRTFDescription: TStringList;
begin
  Result.Text := '';
  Result.Text := richDescription.Lines.Text;
end;

But that doesn't work either.

+12  A: 

The compiler is correct. Result is not initialized by default. Try

function TfPackagedItemEdit.GetRTFDescription: TStringList;
begin
  Result = TStringList.Create();
  Result.Text := richDescription.Lines.Text;
end;

Update: After reviewing the comments I believe the original poster actually wants something like this.

function TfPackagedItemEdit.GetRTFDescription: String;
begin
  Result := richDescription.Lines.Text;
end;
Lawrence Barsanti
And don't forget that this means that the caller becomes responsible for freeing the memory that was allocated by GetRTFDescription.
Scott W
I'd like a clarification on that. How would I free the memory? I've just been calling this as a property getter....
croceldon
Also, use "Result.Assign(richDescription.Lines);" for speed, the code as it is concatenates all lines into one string, only to split the string back into lines.
mghie
Croceldon, calling this function gives you a BRAND NEW TStringList instance, just as if you'd called TStringList.Create. You free it the same way you free any other object: Call Free on it.
Rob Kennedy
But I'm not calling the function directly, I'm just accessing a property (RTFDescription). So, in my code, I'd have to do fPackagedItemEdit.RTFDescription.Free?
croceldon
@croceldon, the property just "wraps" the call to the function in this case, so yes, you would need to Free the resulting TStrings yourself. If you'd like to avoid that, Rob Kennedy's answer below just returns the same TStrings instance, so you won't need to free it yourself.
Iceman
"GetXxx" is a really bad name for a class factory what this function is. I would use "CreateXxx" instead because that automatically indicates that the caller is resposible for calling "Free".
Andreas Hausladen
No, Croceldon, calling "fPackagedItemEdit.RTFDescription.Free" would NOT accomplish anything. Accessing the RTFDescription property a second time would create a second TStringList object. The first one would still remain unfreed.
Rob Kennedy
+16  A: 

The Result variable is not initialized by default. It doesn't automatically refer to some compiler-generated TStringList instance. You need to assign a value to Result. That means having a line like this somewhere in your code:

Result := ...;

An expression like Result.X is reading the value of Result in order to get a reference to its X member, so you need to have given Result a value already. Larry's answer demonstrates how to do that. It generates a new TStringList instance, so the caller of this function needs to call Free on that object sometime.

But in a comment, you mention that you're using this function as a property accessor. It's inconvenient for callers to have to free objects every time they read a property, so your whole plan might be inappropriate. Since it looks like you're trying to expose the description text, you might want to consider this instead:

function TfPackagedItemEdit.GetRTFDescription: TStrings;
begin
  Result := richDescription.Lines;
end;

Notice first that I've changed the return type to TStrings, which is essentially the abstract base class of all kinds of string lists throughout the VCL. TStringList is one descendant, but TRichEdit.Lines doesn't use TStringList. Instead, it uses a specialized TStrings descendant that knows how to interact with the underlying rich edit control.

Next, notice that I have not created any new objects. Instead, I have returned a reference directly to the control's Lines property. Users of your RTFDescription property no longer need to worry about freeing the object they get.

Rob Kennedy
For safety purposes, I would also add a check to insure that richDescription is also assigned, just to protect against the case when GetRTFDescription is called and richDescription has not yet been created. Easier to check for nil after calling GetRTFDescription than to trap for exceptions which could easily be handled.
skamradt
That's the sort of thing an assertion is for. It's assumed that all internal components are assigned before anyone starts messing with properties. If they're not, then it's programmer error, not user error, and should be fixed before shipping.
Rob Kennedy
+1  A: 

I agree that returning a simple String would probably be the best solution, or at least it'll be friendly to use.

Alternatively, you could return the reference to the TStringList, which is the fastest solution if you're having a lot of text..

function TfPackagedItemEdit.GetRTFDescription: TStringList;
begin
  Result := richDescription.Lines; 
end;
Wouter van Nifterick