:)
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?
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.
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.
For I := stringlist.count-1 downto 0 do
Now you can delete all items without any error
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.
you may change the for
loop into a while
loop that reevaluates count
and don't increment index on the delete iteration
you may reverse the loop, kinda for i:=count downto 1
instead of delete
, you may create a temporary list and copy there only the items you want to keep, and recopy it back.
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;