Delphi - Loop through the String

Question

I'm trying to find out if String is "mnemonic type"... My mnemonic type consists of letters from 'a' to 'z' and from 'A' to 'Z', digits from '0' to '9', and additionaly '_'. I build code like below. It should result with True if given string match my mnemonic pattern otherwise False:

 TRes := True;
 for I := 0 to (AString.Length - 1) do
 begin
     if not ((('0' <= AString[I]) and (AString[I] <= '9')) 
       or (('a' <= AString[I]) and (AString[I] <= 'z')) 
       or (('A' <= AString[I]) and (AString[I] <= 'Z')) 
       or (AString[I] = '_')) then
         TRes := False;
 end;

This code always results with False.

Answer 1

I'm assuming that since you tagged the question XE5, and used zero-based indexing, that your strings are zero-based. But perhaps that assumptions was mistaken.

Your logic is fine, although it is rather hard to read. The code in the question is already doing what you intend. At least the if statement does indeed perform the test that you intend.

Let's just re-write your code to make it easier to understand. I'm going to lay it our differently, and use a local loop variable to represent each character:

for C in AString do
begin
  if not (
        (('0' <= C) and (C <= '9'))  // C is in range 0..9
     or (('a' <= C) and (C <= 'z'))  // C is in range a..z
     or (('A' <= C) and (C <= 'Z'))  // C is in range A..Z
     or (C = '_')                    // C is _
  ) then
    TRes := False;
end;

When written like that I'm sure that you will agree that it performs the test that you intend.

To make the code easier to understand however, I would write an IsValidIdentifierChar function:

function IsValidIdentifierChar(C: Char): Boolean;
begin
  Result :=  ((C >= '0') and (C <= '9'))
          or ((C >= 'A') and (C <= 'Z'))
          or ((C >= 'a') and (C <= 'z'))
          or (C = '_');
end;

As @TLama says, you can write IsValidIdentifierChar more concisely using CharInSet:

function IsValidIdentifierChar(C: Char): Boolean;
begin
  Result := CharInSet(C, ['0'..'9', 'a'..'z', 'A'..'Z', '_']);
end;

Then you can build your loop on top of this function:

TRes := True;
for C in AString do
  if not IsValidIdentifierChar(C) do 
  begin
    TRes := False;
    break;
  end;