5

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.

Improve this question
0

2 Answers 2

Reset to default
13

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;
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Or Result := CharInSet(C, ['0'..'9', 'a'..'z', 'A'..'Z', '_'); instead of that ugly mix of operators :)
I missed {$ZEROBASEDSTRINGS}. Thanks for the rest of the comments.
Yeah, I just assumed that you were on a mobile platform. Should have been more careful. The for in loop is your friend here!
Hi, I repaired question, please check if I could get some "+" points :)
7

String type is 1-based. dynamic Arrays are 0-based. Better use for ... in so you are safe for future Delphi's.

Testing for ranges of possible character values can be done more efficiently (and more conciece) is CharInSet.

function IsMnemonic( AString: string ): Boolean;
var
  Ch: Char;
begin
  for Ch in AString do 
    if not CharInSet( Ch, [ '_', '0'..'9', 'A'..'Z', 'a'..'z' ] ) then 
      Exit( False );
  Result := True;
end;
Improve this answer

7 Comments

String can be zero based as well, $ZEROBASEDSTRINGS ON.
This is XE5. My assumption is that we are using zero based strings. Perhaps that was naive of me.
@LU RD, thats why the for in is a better solution. I thought not every compiler in the XE5 suite supports the 0/1 based option (but can be wrong).
It is possible to use for i := Low(AString) to High(AString) as well.
@LURD: only in compiler versions that support {$ZEROBASEDSTRINGS}.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.