Are there good practices if any avoiding out of bo

Question

:)

First thing, my code

procedure TForm1.Button3Click(Sender: TObject);
var tempId,i:integer;
begin
tempId:=strtoint(edit5.Text);
plik:=TStringList.Create;
plik.LoadFromFile('.\klienci\'+linia_klient[id+1]+'.txt');
if (plik.Count=1) then
  begin
  label6.Caption:='then';
    if (tempId=StrToInt(plik[0])) then
      begin
      Label6.Caption:='Zwrócono';
      plik.Delete(0);
    end
  end
else
for i:=0 to plik.Count-2 do
  begin
    if (tempId=StrToInt(plik[i])) then
    begin
      Label6.Caption:='Zwrócono';
      plik.Delete(i);
    end;
  end;
plik.SaveToFile('.\klienci\'+linia_klient[id+1]+'.txt');
plik.Free;
end;

• When for i:=0 to plik.Count-2 do I can delete any element but not last.
• When for i:=0 to plik.Count-1 do I can delete any element without but from end to start. Because otherwise List index out of bounds.

What's going one? How can I safety search and remove elements from TStringList?

Answer 1

This is a classic problem. A for loop evaluates the loop bounds once at the beginning of the loop, so you run off the end which explains your index out of bounds errors.

But even if for loops evaluated loop bounds every time like a while does that would not really help. When you delete an element, you reduce the Count by 1 and move the remaining elements down one in the list. So you change the index of all those still to be processed elements.

The standard trick is to loop down the list:

for i := List.Count-1 downto 0 do
  if DeleteThisItem(i) then
    List.Delete(i);

When you write it this way, the call to Delete affects the indices of elements that have already been processed.

Answer 2

For I := stringlist.count-1 downto 0 do

Now you can delete all items without any error

Answer 3

in an ascending loop like for i:=1 to count you just can't delete items of the list you are iterating over.

there are several solutions depending on the overall logic of what you want to achieve.

1. you may change the for loop into a while loop that reevaluates count and don't increment index on the delete iteration

2. you may reverse the loop, kinda for i:=count downto 1

3. instead of delete, you may create a temporary list and copy there only the items you want to keep, and recopy it back.

Answer 4

When deleting intems from list you want to use downto loop, ie

for i := plik.Count-1 downto 0 do
  begin
    if (tempId=StrToInt(plik[i])) then
    begin
      Label6.Caption:='Zwrócono';
      plik.Delete(i);
    end;
  end;

This ensures that if you delete item, the loop index stays valid as you move from the end of the list dowards beginning of the list.

Answer 5

As others have said, using a downto loop is usually the best choice. Of course, it does change the semantics of the loop so it runs backwards instead of forwards. If you want to continue looping forwards, you have to use a while loop instead, eg:

I := 0;
while I < plik.Count do 
begin 
  if (tempId = StrToInt(plik[I])) then 
  begin 
    ...
    plik.Delete(I); 
  end else
    Inc(I); 
end; 

Or:

var
  CurIdx, Cnt: Integer;

CurIdx := 0;
Cnt := plik.Count;
for I := 0 to Cnt-1 do 
begin 
  if (tempId = StrToInt(plik[CurIdx])) then 
  begin 
    ...
    plik.Delete(CurIdx); 
  end else
    Inc(CurIdx); 
end;