Amazon.com Widgets Fun Code of the Week #4

Fun Code of the Week #4

By Nick at June 08, 2012 15:14
Filed Under: Delphi, Fun Code

Adapted from my answer on StackOverflow:

function RemoveChars(const aString: string; aCharsToRemove: TSysCharSet): string;
var
  C: Char;
begin
  Result := '';
  for C in aString do
  begin
    if not CharInSet(C, aCharsToRemove) then
    begin
      Result := Result + C;
    end;
  end;
end;

Comments (15) -

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 Chile |

6/8/2012 4:47:08 PM #

Yes, I can -- by why would I?  Wink

Nick Hodges United States |

6/8/2012 5:19:47 PM #

To leave only the 3 lines that matter and remove the superfluous 4 that just bring visual clutter in an otherwise very clear snippet...

François United States |

6/8/2012 6:14:35 PM #

Exactly.

Rodrigo Chile |

6/9/2012 11:29:00 AM #

I am of the opinion that a begin...end pair cannot be clutter, and can only increase clarification.

Nick Hodges United States |

6/10/2012 9:03:50 PM #

Yes i never write my code without begin .. end

Bagus Prasojo Indonesia |

6/11/2012 1:03:27 PM #

I have only benchmarked this on earlier delphi compilers, (delphi 5, 6, 7) but when you add begin end to single line statements you get extra code block setup in the compiled code.

I.E. a Data pump I built that had to push a 1 gig csv into a custom DB format, the processing time went from 3 hours to 25 minutes. (granted there were other modifications but the majority was single statement loops and other single statement code blocks) This was performed on a AMD k2 500, Yes it has been a while.......

Jeremy Evans United States |

6/8/2012 11:12:57 PM #

I am with you on this one Nick. I bracket single lines with begin...end to avoid the possibility of unexpected program flow if I pop in another line later on.  Plus with Castalia's structural highlighting I actually find it easier to read.  It does make the code longer and if you have very large number of IFs in one section it gets close to not being worth it due to the bloat but I put up with it for higher reliability in the code.

Leonard United States |

6/9/2012 11:29:39 AM #

"to avoid the possibility of unexpected program flow if I pop in another line later on. "

Exactly.  It's defensive.

Nick Hodges United States |

6/11/2012 1:45:53 AM #

BTW, that's exactly the argument why I use FreeAndNil: it's defensive. Begin-end statements around single lines don't change anything in the compiled code so they are only clutter to me. This is, however, just my personal opinion.

Joe Germany |

6/23/2012 3:23:25 AM #

FreeAndNil is more than defensive.  It relies on RTTI to ferret out what the object's type is.  That is, it has more overhead than myObj.Free.  And, what's the point of assigning nil to a pointer in a local stack frame that's about to be zapped anyway?  Just more useless overhead.

David S United States |

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 Argentina |

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 Netherlands |

6/9/2012 11:30:41 AM #

Phew!  Thanks for the code.  I was getting a bit worried that I'd do a Fun Code posting and that no one would try to optimize it!  Wink

Nick Hodges United States |

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 United States |

Comments are closed

My Book

A Pithy Quote for You

"Courage is not simply one of the virtues, but the form of every virtue at its testing point."    –  C. S. Lewis

Amazon Gift Cards

General Disclaimer

The views I express here are entirely my own and not necessarily those of any other rational person or organization.  However, I strongly recommend that you agree with pretty much everything I say because, well, I'm right.  Most of the time. Except when I'm not, in which case, you shouldn't agree with me.

Month List