CODEWITHSUNDEEP
delphi
Franz
Franz

Reputation: 2023

compact delphi code , handling TStringlist as parameter

the following line of code fails :

... 
ProcessStringLIst(ChecklistBox_CheckedStrings(MyCheckListBox));        
....

//  support functions

function ProcessStringLIst (alst : TStringlist);
begin
      ///  ....  process the stringlist 

end;

function ChecklistBox_CheckedStrings(aCheckListBox: TCheckListBox): TStringList;
var
  i: Integer;
begin
  result.Clear;
  for i := 0 to aCheckListBox.Items.Count - 1 do
    if aCheckListBox.Checked[i] then
      result.Add(aCheckListBox.Items[i])
end;

because inside ChecklistBox_CheckedStrings the result is not yet assignd with data . Can I avoid the 4 line version as below : 

 templist := TStringlist.Create;
 temList := ChecklistBox_CheckedStrings(MyCheckListBox);
 ProcessStringLIst(templist);
 templist.free;

Upvotes: 2

Views: 2026

Answers (1)

David Heffernan
David Heffernan

Reputation: 612794

In your code the function return value is not initialized, and then your first line of code does:

result.Clear;

Because you have not initialized result, anything can happen. If you enable compiler warnings then the compiler will tell you this.

You need to make up your mind whether or not you want the function to return a newly created string list object, or to work with one created by the caller. Let's assume you opt for the latter. Then your function becomes a procedure like this:

procedure GetChecklistBox_CheckedStrings(aCheckListBox: TCheckListBox; 
  aStringList: TStringList);
var
  i: Integer;
begin
  aStringList.Clear;
  for i := 0 to aCheckListBox.Items.Count - 1 do
    if aCheckListBox.Checked[i] then
      aStringList.Add(aCheckListBox.Items[i])
end;

The calling code then becomes:

templist := TStringlist.Create;
try
  GetChecklistBox_CheckedStrings(MyCheckListBox, templist);
  ProcessStringList(templist);
finally
  templist.free;
end;

You must use try/finally if you want to protect your program against memory leaks.

And the other way looks like this:

function CreateCheckListBox_CheckedStrings(aCheckListBox: TCheckListBox): TStringList;
var
  i: Integer;
begin
  Result := TStringList.Create;
  try
    for i := 0 to aCheckListBox.Items.Count - 1 do
      if aCheckListBox.Checked[i] then
        Result.Add(aCheckListBox.Items[i])
  except
    Result.Free;
    raise;
  end;
end;

.....

templist := CreateCheckListBox_CheckedStrings(MyCheckListBox);
try
  ProcessStringList(templist);
finally
  templist.free;
end;

Again, it's easy to put either of these methods into a class helper to make the calling syntax look more clear.

My most important pieces of advice:

  1. Enable warnings and hints and make sure your code never has any.
  2. Use try/finally correctly.

Upvotes: 4

Related Questions