6/8/2012 3:46:20 PM #

Nick you can remove the two inner begin - end blocks an just write

  for C in aString do
    if not CharInSet(C, aCharsToRemove) then
      Result := Result + C;

Rodrigo |

6/8/2012 4:00:15 PM #

Rodrigo,
  Yes, he can. But he is writing pascal well formatted code. Your code, seems a lot like C code. Please dont make people lost good habits!

donald shimoda |

6/8/2012 11:43:42 PM #

Hi Nick,

I know that speed or performance was not the goal of your code, but if anyone is to actually going to use this code, I'd suggest using a faster version, like this:

function RemoveNumbers2(const aString: string; aCharsToRemove: TSysCharSet): string;
var
  C:Char; Index:Integer;
begin
  Result := '';
  SetLength(Result, Length(aString));
  Index := 1;
  for C in aString do
    if not CharInSet(C, aCharsToRemove) then
    begin
      Result[Index] := C;
      Inc(Index);
    end;
  SetLength(Result, Index-1);
end;


I've noticed some unintuitive performance behavior while I was making changes:

1. I've tried to rewrite `for c in aString` to `for i:=1 to Length(aString)`, but it became almost twice as slow. Maybe it's because the for..in doesn't trigger any range checking? Maybe I should try compiling the for..to version with range checking turned off.

2. Passing the TSysCharSet as a parameter to the function instead of having it as a hardcoded constant (like your version on StackOverflow) made the function about 15% faster on my machine.

3. Passing the TSysCharSet as a const parameter made the function slower. I don't really get why that would be.

Wouter van Nifterick |

6/9/2012 9:04:27 AM #

Maybe it is better to Reverse Thinking -  consider the Delete approach. E.g. For a string which contains 255 letters with 2 candidates, The concat operation is 253 times while using Delete it will be performed 2...

Paul |