Wrong data moving from TListbox to TStringGrid

Question

I am working on Delphi 7. I have one TListBox and one TStringGrid with two columns (with no fixed row or column). I have data in the TListBox as follows:

Available Elements - a123 (a123) Available Elements - a1234 (a1234) Available Elements - a12345 (a12345)

And the TStringGrid is having following data as follows:

Column1 Column2

a1 Available Elements - a1 a2 Available Elements - a12

If I select the first item in the TListbox i.e. a123 and execute following button click event procedure, then the last item data ie a12345 is getting moved into the grid.

Could anybody put the focus on what I am doing wrong in the following code. Following code moves the seleted item in the TListbox to TStringgird's two columns:

procedure TForm1.btnMoveLeftClick(Sender: TObject);
var
  sString : String;
  i : Integer;
begin
  for i := 0 to ListBox1.Items.Count - 1 do
  begin
      {-- Is this status selected? --}
      if ListBox1.Selected[i] then
      begin
        sString := Trim(ListBox1.Items[i]);

        {-- Delete selected status. --}
          ListBox1.Items.Delete (i);

        if ((grdVFormDetails.RowCount >= 1) And (Trim(grdVFormDetails.Cells[0, 0]) <> EmptyStr)) then
          grdVFormDetails.RowCount := grdVFormDetails.RowCount+1;

        grdVFormDetails.Cols[1].Add(Copy(sString, 1, Pos('(', sString) - 1));

        sString := Copy(sString, Pos('(', sString) + 1, Length(sString));
        sString := Copy(sString, Pos('(', sString) + 1, Length(sString) - 1);

        grdVFormDetails.Cols[0].Add(sString);


        break;
      end;
  end;
end;

Answer 1

NEVER delete Items of TList in the FOR loop.

The issue in this line:

  ListBox1.Items.Delete (i);

The loop goes from i:=0 to 2. Item - 0 is selected and you delete it. What we got on the next repeat? i=1 but here are only 2 items left instead of 3 (all following items shifted) and i points at the last item not the second. On the next repeat when i=3 we will get "Index out of bound" error. You should delete Item only after FOR loop to avoid this issue.

procedure TForm1.btnMoveLeftClick(Sender: TObject);
var
  sString : String;
  i : Integer;
  k: integer;
begin
  k:=-1; 
  for i := 0 to ListBox1.Items.Count - 1 do
  begin
      {-- Is this status selected? --}
      if ListBox1.Selected[i] then
      begin
        sString := Trim(ListBox1.Items[i]);

        {-- Delete selected status. --}
          k:=i;         

        if ((grdVFormDetails.RowCount >= 1) And (Trim(grdVFormDetails.Cells[0, 0]) <> EmptyStr)) then
          grdVFormDetails.RowCount := grdVFormDetails.RowCount+1;

        grdVFormDetails.Cols[1].Add(Copy(sString, 1, Pos('(', sString) - 1));

        sString := Copy(sString, Pos('(', sString) + 1, Length(sString));
        sString := Copy(sString, Pos('(', sString) + 1, Length(sString) - 1);

        grdVFormDetails.Cols[0].Add(sString);


        break;
      end;
  end;
  if k>=0 then  ListBox1.Items.Delete (k);

end;

Answer 2

Assuming, you want to parse the input string like this:

'Some text (comment) etc. (12345)'

into a part with a trimmed string from the beginning to the first opening parenthesis (first from the end) of the input string to get a value like this:

'Some text (comment) etc.'

and the string from inside the last parentheses of the input string:

'12345'

If so, you can use the code that follows. Note that is expected to have list box items terminated by closing parentheses. You may check the commented version of this code or download a sample project if you want.

Here is the part, which moves the focused item from the list box to the string grid:

procedure TForm1.MoveLeftButtonClick(Sender: TObject);
var
  S: string;
  I: Integer;
  ItemID: string;
  ItemText: string;
begin
  if ListBox1.ItemIndex = -1 then
    Exit;

  S := ListBox1.Items[ListBox1.ItemIndex];
  for I := Length(S) - 1 downto 1 do
  begin
    if S[I] = '(' then
    begin
      ItemID := Trim(Copy(S, I + 1, Length(S) - I - 1));
      ItemText := Trim(Copy(S, 1, I - 1));
      with StringGrid1 do
      begin
        if (Cells[0, RowCount - 1] <> '') and
          (Cells[1, RowCount - 1] <> '')
        then
          RowCount := RowCount + 1;
        Cells[0, RowCount - 1] := ItemID;
        Cells[1, RowCount - 1] := ItemText;
      end;
      ListBox1.Items.Delete(ListBox1.ItemIndex);
      Break;
    end;
  end;
end;

And here is the part which moves the selected row from the string grid to the list box:

procedure TForm1.MoveRightButtonClick(Sender: TObject);
var
  I: Integer;
  RowIndex: Integer;
begin
  RowIndex := StringGrid1.Selection.Top;
  if (StringGrid1.Cells[0, RowIndex] <> '') and
    (StringGrid1.Cells[1, RowIndex] <> '') then
  begin
    ListBox1.Items.Add(
      Trim(StringGrid1.Cells[1, RowIndex]) + ' (' +
      Trim(StringGrid1.Cells[0, RowIndex]) + ')'
    );
    for I := RowIndex to StringGrid1.RowCount - 2 do
      StringGrid1.Rows[I].Assign(StringGrid1.Rows[I + 1]);
    if StringGrid1.RowCount > 1 then
      StringGrid1.RowCount := StringGrid1.RowCount - 1
    else
    begin
      StringGrid1.Cells[0, 0] := '';
      StringGrid1.Cells[1, 0] := '';
    end;
  end;
end;

Answer 3

I do not have Delphi installed at the moment, though as far as I remember, if ListBox1.Selected[i] then will not give you correct result. You need to get the item firstly, them check if the item is selected.

Answer 4

If you don't have multi-select in the ListBox, better use the ItemIndex property, which gives you the Index of the selected Item, thus you don't have to loop to check all the Items in the list.

Like this:

procedure TForm1.btnMoveLeftClick(Sender: TObject);
var
  sString : String;
  i : Integer;
begin
  if ListBox1.ItemIndex <> -1 then
  begin
    sString := Trim(ListBox1.Items[ListBox1.ItemIndex]);
    //do all your processing here
    //and at the end:
    ListBox1.Items.Delete(ListBox1.ItemIndex);
  end
end;