C99bb2b0dfcf7dba9aabf244a3330ac2

I often find myself copying the following code and making changes to it when i want to search for something in a TList

can anyone see if it can be improved/speed up?

TMylist = class(TList);

Thankyou.

(delphi 7 code would be nice)

function TMylist.GetNameIndex(aName: Sting): integer;
var
  I:     integer;
begin
  i := 0;
  result := -1;

  while (i < Count - 1) and (not sametext(Items[i].name = aName)) do
      Inc(i);

  if sametext(Items[i].name = aName) then
    Result := i
end;


function TMylist.GetItems(Index: integer): TMylist;
begin
  Result := TMylist(inherited Items[index]);
end;

Refactorings

No refactoring yet !

7983135a9fcc8bcfe9d9de240225512a

ahuser

February 12, 2009, February 12, 2009 11:52, permalink

3 ratings. Login to rate!

The following code should be slightly faster. It removes the try/finally that the compiler injects into your code because of the missing "const" for the string parameter. And it has only one function call in the loop. But the main problem are the string comparisons in the loop.
For this you could either sort the list and then access if via a binary search algorithm, or you could hash the string values and compare the hashes first before performing the costly string comparison.
The binary search can only be used if you are not interested in the order of the list items. And using a hash would require an additional field (the hash value) for each item (preferable placed into the item's class).

function TMylist.GetNameIndex(const aName: string): integer; // use "const" for strings to eliminate the injected try/finally
var
  LocalList: PPointerList;
begin
  LocalList := List; // use a local variable, removes one indirection, no bound checks required
  for Result := 0 to Count - 1 do
    if SameText(TMylist(LocalList[Result]).name, aName) then // do not call GetItems
      Exit;
  Result := -1;
end;

function TMylist.GetItems(Index: integer): TMylist;
begin
  Result := TMylist(inherited Items[index]);
end;
7983135a9fcc8bcfe9d9de240225512a

ahuser

February 12, 2009, February 12, 2009 17:32, permalink

2 ratings. Login to rate!

And this is the code that uses a hash value to omit many string comparisons. But it requires an additional field (FHashValue: THashValue) that must be recalculated after the FName field was changed.

type
  THashValue = Cardinal;

  TMylist = class(TList)
  private
    FName: string;
    FHashValue: THashValue;
  public
    constructor Create(const AName: string);
    function GetNameIndex(const aName: string): integer;
    function GetItems(Index: integer): TMylist;

    property Name: string read FName;
  end;

function NonAsciiHashName(aName: string): THashValue;
var
  I: Integer;
begin
  aName := AnsiLowerCase(aName);
  Result := 0;
  for I := 1 to Length(aName) do
    Inc(Result, Ord(aName[I]));
end;

function HashName(const aName: string): THashValue;
var
  Len: Integer;
  Ch: Char;
  P: PChar;
begin
  Len := Length(aName);
  P := Pointer(aName);
  Result := 0;
  while Len > 0 do
  begin
    Ch := P[0];
    if (Ord(Ch) and {$IFDEF UNICODE}$FF80{$ELSE}$80{$ENDIF}) <> 0 then
      Break;
    Inc(Result, Ord(Ch) or $20);
    Inc(P);
    Dec(Len);
  end;
  if Len > 0 then
    Result := NonAsciiHashName(aName);
end;

{ TMylist }

constructor TMylist.Create(const aName: string);
begin
  inherited Create;
  FName := aName;
  FHashValue := HashName(AName);
end;

function TMylist.GetNameIndex(const aName: string): integer;
var
  LocalList: PPointerList;
  HashValue: THashValue;
begin
  HashValue := HashName(aName);
  LocalList := List;
  for Result := 0 to Count - 1 do
    if (HashValue = TMylist(LocalList[Result]).FHashValue) and
       (SameText(TMylist(LocalList[Result]).Name, aName)) then
      Exit;
  Result := -1;
end;

function TMylist.GetItems(Index: integer): TMylist;
begin
  Result := TMylist(inherited Items[index]);
end;

Your refactoring





Format Copy from initial code

or Cancel