Delphi for loops and StringList Errors

Gab picture Gab · Nov 22, 2011 · Viewed 11.8k times · Source

Ok guys, I've been trying to find out every possible mistake i'm making but I give up... I need help! What I'm writing is an app to manage rentals for my job and when the date is past, my app removes the name from 2 text files. I wrote 3 little functions(procedures) to make this work. Here:

This one loads from dates.dat file and remove the line containing the name of the employee.

procedure remDate(emp: String);/// Removes employee from date file
var
  pos1, i: integer;
  dateList: TStringList;
begin
  dateList:=TStringList.Create;
  dateList.LoadFromFile('Data\dates.dat');
  for i:=0 to dateList.Count-1 do begin
    pos1:=AnsiPos(emp, dateList[i]);
    if pos1<>0 then begin
      dateList.Delete(i);
      dateList.SaveToFile('Data\dates.dat');
    end;
  end;
  dateList.Free;
end; //eo remDate

This one removes the line containing the employee name from the perm.dat file.

procedure remPerm(emp: String);/// Removes employee from perm file
var
  pos1, i: integer;
  permList: TStringList;
begin
  permList:=TStringList.Create;
  permList.LoadFromFile('Data\perm.dat');
  for i:=0 to permList.Count-1 do begin
    pos1:=AnsiPos(emp, permList[i]);
    if pos1<>0 then begin
      permList.Delete(i);
      permList.SaveToFile('Data\perm.dat');
    end;
  end;
  permList.Free;
end; //eo remPerm

This one sticks those together. The isDue is a simple function that compares 2 dates and returns a TRUE if date is today or is past.

procedure updatePerms;
var
  empList: TStringList;
  i: integer;
begin
  empList:=TStringList.Create;
  empList.LoadFromFile('Data\employes.dat');
  for i:=0 to empList.Count-1 do begin
    if isDue(empList[i]) then begin
      remDate(empList[i]);
      remPerm(empList[i]);  (*) Here is where the error points.
    end;
  end;
  empList.Free;
end;

The error I get is when it gets to remPerm in the updatePerms procedure.(*) I get a EStringList Error, out of bound (#). Figured out with many tries that it only happens when an employee's due date is today. Please comment if you need more info! Thanks in advance, any help is really appreciated!

Answer

Gerry Coll picture Gerry Coll · Nov 22, 2011

The problem is that you are using a for loop. The end point of a for loop is only evaluated once when the loop is entered. At that point you may have 100 items, but once you start deleting there will be less. This will then result in a list index out of bounds error.

The simple fix is to reverse the for loop:

procedure remDate(emp: String);
/// Removes employee from date file
var
  pos1, i: integer;
  dateList: TStringList;
begin
  dateList := TStringList.Create;
  dateList.LoadFromFile('Data\dates.dat');
  for i := dateList.Count - 1 downto 0 do
  begin
    pos1 := AnsiPos(emp, dateList[i]);
    if pos1 <> 0 then
    begin
      dateList.Delete(i);
      dateList.SaveToFile('Data\dates.dat');
    end;
  end;
  dateList.Free;
end; // eo remDate

This will work if the employee occurs more than once.

However if the employee does only occur once, you can use break to exit from the loop early:

procedure remDate(emp: String);
/// Removes employee from date file
var
  pos1, i: integer;
  dateList: TStringList;
begin
  dateList := TStringList.Create;
  dateList.LoadFromFile('Data\dates.dat');
  for i := 0 to dateList.Count - 1 do
  begin
    pos1 := AnsiPos(emp, dateList[i]);
    if pos1 <> 0 then
    begin
      dateList.Delete(i);
      dateList.SaveToFile('Data\dates.dat');
      Break; // <-- early exit
    end;
  end;
  dateList.Free;
end; // eo remDate

Another solution is to use a while loop.